Good Coding Strategies for Common Vulnerabilities?

[consultant1027]

I've got a few sites with older code that we need to patch for various security holes and I'm trying to determine the most efficient strategies for doing it.  The three most common vulnerabilities we are currently focusing on are:

 

1) XSS Vulnerability on User Form Input

 

Notes: Does someone want to explain to me why this is even that big a deal.  This involves inserting Javascript code into form inputs that re-display the input.  I'm unclear how this could compromise security on the server or for any other users other than the one viewing the page?  Wouldn't this require some sort of a trojan on the client-side to really do anything or is there some JS code you could inject that would actually modify the server's pages for all users?

 

Anyway I came across a well writting sanitizing function to filter XSS Javascript code from user input and just run all input through that function prior to processing it.  I can't think of a simpler, easier way to address it site wide.

 

Would there be a problem with writing a function that instead of santizing a passed value, it simply sanitizes ALL of the $_POST and $_GET values in a PHP script at the beginning of the script so you don't have to bother sanitizing each value individually?

 

 

2) SQL Injection - (inserting SQL syntax into user form input to modify a SQL query)

 

Out of all the most common serious exploits, this is by far the most common.  This is easily prevented by sanitizing all user input that could be used in a SQL query with something like the mysql_real_escape_string function in PHP, although it doesn't cover every single possibility as it does not allow for % characters if I'm correct.  So I believe the way to be fullproof is to actually write your own sanitizing function?  I'm curious what other methods people have implied?  It would seem easier if you could just build your query then sanitize the query instead of the values, but I don't think that's possible as the sanitize function wouldn't know what is the legitimate query and what is the injection.

 

The big question is, can there be a function which essentially does both without any harm, sanitizes all $_GET and $_POST values for XSS and SQL injection?  It seems there must be some cases though where a legitimate value might get corrupted in the sanitization process but I can't think of any examples off hand.

 

3) URL modification

 

This is more of a security logic issue than a universal exploit.  An example would be if a UserID is passed in the URL and you modify it to another ID and it brings up the other users's information.  In that case the solution is obviously to only pass the userID in a cookie.  Or if there is say, an entity ID for something that 'belongs' to that user and you modify it to see the entity that belongs to another user.  In that case you must check the cookie with the UserID and make sure it matches the entity owner's user ID.  Doesn't seem like you can apply a single universal function to handle URL modification.  Needs to be done on a case by case basis.  Maybe though in the URL you could pass a hash of the value you don't want tampered with then check the hash?  That seems like overkill.  Seems URL modification has to be handled on a case by case basis.

 

There's actually a 4th issue.

 

4) Hidden form values

 

Some sites may pass sensitive information from page to page via a hidden form value, such as a password.  This seem like a major problem on the face of it, but let's think about this a little more.  The hidden value is only visible to the user of the client machine by virtue of viewing the HTML source.  The only way it would be visible to another user is if someone else used the same machine and viewed the same page prior to the previous user's session ending.  So say you made sure you removed the hidden values.  Say you went even further to put a no-cache directive in the page so the back button wouldn't bring up the page AND you made sure there was like a 5 minute login timeout.  That's essentially what many online banking sites do I guess.  But still, if someone leaves their computer and another person sits down prior to the timeout, you've still got a problem.  No 100% solution here.  In fact, if a password was being passed as a hidden variable, someone would have to hope they left the PC and be smart enough to sit down and view the HTML source.  Ya, like that's a really common occurence?! 

 

So would the best solution be to simply use an md5 hash of the actual value if you are going to pass the value in the hidden form.  Seems better than storing it in a cookie, since someone could read the cookie values on a person's machine.  Bottom line is always never store highly sensitive data in a cookie (or pass it in a hidden form variable) but it seems the cookie method is much more insecure, since the cookie file can remain on the person's machine indefinitely.

 

I'd appreciate any feedback if anyone has some slick solutions or good points about these security issues.

 

(Or maybe just a URL to a site that is the authority on handling this stuff. - sorry I'm really busy and don't have a lot of time to search all night - I type faster than I read

 

Are there any other major/common exploits that commonly get overlooks when coding sites I'm not discussing?

 

[Stooney]

XSS - say someone is updating their profile and for their email address they put <script src="http://somwhere.com/bad.jss"></script>.

Well now whoever sees their email address will have bad.jss attempting to run.  And generally you don't have to sanitize their input, just restrict the characters they can use in the first place depending on the fields, then output it all with htmlspecialchars($data). 

 

As for SQL injections, I don't think mysql_real_escape_string blocks %, but I may be wrong.

 

As for url modification, just have your pages check the users permission.  If they are allowed to edit profile id 12, but change it 15, the script will simply see they are not allowed access to it and no problem.  Don't believe any variables passed through the url, double check the values.  Never trust the end user.  Assume everyone is trying their hardest to get passed your security.

 

Hidden Form Values - If the variable's value is sensitive, use sessions.  And again don't trust the end result once it reaches it's destination.  Double check that it's value is what you expect.

 

Another tip, store your mysql database connection file above the webroot.  This way nobody can remotely include the file on another site and gain access to your database.

[consultant1027]

Ya, I see the point about XSS but that's not going to effect the website in question, just the client machine.  At it is the user that is responsible for maintaining their client machine (running anti-virus and spyware protection, etc.).  But I suppose it would be a little embarassing if someone did get infected a traced it to an XSS exploit on a major site, like cnn.com or something.  It doesn't compromise your server, so your just looking out for the unprotected users out there.  Correct me if I'm wrong.

 

 

 

[consultant1027]

I've heard using prepared statement is the best (cleanest, most fullproof) way to prevent SQL injection. 

 

I've also read:

 

"setting the magic_quotes_gpc php.ini variable to Off, will automatically apply addslashes to all values submitted via GET, POST or cookies. This feature safeguards against inexperienced developers who might otherwise leave security holes like the one described above, but it has an unfortunate impact on performance when input values do not need to be escaped for use in database queries. Thus, most experienced developers elect to switch this feature off."

 

The question is will turning magic quotes off, have some other bad side effects other than performance, and is the performance hit really an issue on a fairly underutilised server?  Although we do perform some complex select statements that retrieve thousands of records at times.

 

 

So what is it, go through the code and use prepared statements or just turn magic quotes off?

 

[consultant1027]

Talking to myself...

 

Found this article.  Seems magic quotes ain't so magic.  Taken out of PHP 6, so forget that as an easy solution I guess?

 

http://www.tizag.com/phpT/php-magic-quotes.php

[consultant1027]

There a lot of code snippets out there for sanitizing input for XSS and SQL Injection vulnerabilities.

 

Most codes use mysql_real_escape_string to protect against SQL Injection, although it doesn't sanitize % which means it is not 100% fullproof.

 

For XSS there are lots of code snippets out there but none of them appear to use the new filter extension introduced in PHP 5.1 (which is enabled by default in 5.2).

 

I now am able to protect an entire website for XSS vulnerabilities simply adding this at the top of the script:

 

$_POST = filter_var_array($_POST,FILTER_SANITIZE_STRING);

$_GET = filter_var_array($_GET,FILTER_SANITIZE_STRING);

 

(I don't ever access $_REQUEST)

 

The only downside I can see to this, is that for instance, in  some cases I might want to allow HTML tags to be passed int the input, such as a simple News Article creation tool where I want the user to be able to include HTML tags that point to external images <img> or include links in the article <a>. 

 

There's a very popular input filter class at:

 

http://www.phpclasses.org/browse/package/2189.html

 

It is quite comprehensive and VERY configurable with separate functions for XSS and SQL Injection filters.

 

For example on the XSS (process) filter I can specify exception to the tags it will filter such as <a> and <img>

 

I believe many coders may filter their input one variable at a time.  The fact of the matter SEEMS you can just do it in one fell swoop if you do need to allow HTML tags in ANY of your input at any time.

 

Unfortunatly on SQL Injection you have clean each individual variable, unless you use prepared statements via PHP Data Objects with MySQL (PDO).

 

So I'm doing a reality check as far as the cleanest, simplest way to squash XSS and SQL Injection vulnerabilities.

 

It seems if I need to allow some HTML tags in input, but don't want to sanitize each individual input value, I'll need to use the input_filter class that allows me to specify exceptions as the new PHP built-in filter functions don't have that option.

 

If I don't want to worry about sanitizing individual variables for each MySQL statement, I should just convert all my queries over to use prepared statements.

 

Am I missing anything, or is there a better/easier/cleaner way?

 

[consultant1027]

I suppose if you have a site what has lots of SQL calls in it already, a simpler way to squashing SQL injection would be to write a function that santizes an array, escaping \x00, \n, \r, \, ', ", \x1a and % for any variables used in the SQL statement?  And just run it prior to preparing the statement?  Seems though on a brand new site, it would be best to just start out using prepared statements from the beginning?

[cunoodle2]

Go with PDO statements.  It is really the way that things are going.  I'm working even a step ahead of things.  Here are a few things I'm doing.

 

1.  On sql statments...

I'm going as far as to create 3 different users all with different permissions.  Sound crazy huh?  Well better safe than sorry.  Here is what I have...

 

	// connect with read statement (can only do select statements)
$slect = new PDO('mysql:host=localhost;dbname=db_name', 'dbName_select', 'pass1');

// connect with insert statement (can only do inserts and deletes)
$insrt = new PDO('mysql:host=localhost;dbname=db_name', 'dbName_insert', 'pass2');

// connect with delete statement (can only do delete statements)
$dlete = new PDO('mysql:host=localhost;dbname=db_name', 'dbName_delete', 'pass3');

 

Basically what I've done is created 3 different user for my database all with different permissions.  The $slect connection can ONLY do "select *..." queries.  $insrt can only do inserts/updates and $dlete can only delete.  This basically allows me to have even more control over my code to set it that deletes are only supposed to be happening when I want them too.  No matter how creative the hackers get.

 

For example...When you are allowing a user to log in they are entering data.  IF there is a vulnerability discovered in the future that some how allows a delete statement to then be inserted into a login box I will be safe as the permissions of the DB connection I'm using ONLY allow that particular user to SELECT and NOT delete.  Over kill I know but is it ever really too much???

 

2.  Check your login password combos a few different ways.  IF they ALL pass THEN log the user in.

-I check the username and password in a query to make sure it returns 1 AND ONLY 1 result

-I then check the username and password against the database results again.  For example I do this..

 

<?
//prepare SQL query using pdo statement
$stmt = $slect->prepare("SELECT * FROM tbl_users where name = ? AND password = ?");
$user_info = $stmt->execute(array($username,$pass));
$result = $stmt->fetch(PDO::FETCH_ASSOC);

//check to make sure there was only 1 result
if ($stmt->rowCount() == 1)
{
	//ok now there is only 1 row but is the data actually valid?
	//check username and password against database to make sure they match
	if ($pass == $result["pos_password"] && $username == $result["pos_name"])
	{
		//good login!!
	}

	else
	{
		//bad login!!
	}
}

else
{
	//bad login!!
}
?>

 

3.  Null set and then over clean your data.  The end user is the enemy!!

Here is how my login file begins and then advances along...

<?php
//NULL set all values
$username = NULL;
$pass = NULL;
.....
//take in passed values from post
$username = isSet($_POST['username']) ? $_POST['username'] : NULL;
$pass = isSet($_POST['pass']) ? $_POST['pass'] : NULL;
.....
//clean up data
$username = trim($username);
$pass = md5(trim($pass));
?>

 

Ok.  Thats enough for right now.  I go to bed.