Best way to prevent sql injection?

[ceefour]

@jjshell

why quoted integers would be a good protection against injections attacks?

Actually, you have to escape (which also means quote) “integers” to prevent SQL injection attacks. Quoting by itself is not enough.

However, the fastest and easiest way if you’re dealing with integers is just typecast it (or use intval, it does exactly the same thing), and forego escaping/quoting altogether. And that works with any DBMS. If anybody is trying to SQL inject that integer, well the typecasted integer would probably end up as some arbitrary value (usually 0), but that’s much better than an executable SQL expression.

[tezza]

I haven’t read every single comment thus far but I thought it worth mentioning that this is new http://phpsec.org/
Their security user guide contains a brief segement on SQL injection but I don’t think it adds much to the discussion here. The rest of the article is still worth a read though.

I think a good place to start is by creating a user in the RDBMS with heavy restrictions (particularly against accessing system tables).

I like the bin2hex idea. It’s the simplest one so far but I haven’t seen how you get it back from hex. I think the main drawback is you can’t easily administer/report on data without being inside a script which can will reverse bin2hex(). Can anyone come up with a scenario where this technique could still be exploited by a cracker?
There’s also base64encode/decode. Could base64decode([insert SQL here]) ever produce a result which could exploit base64encode() ?
There is also pack()/unpack() which seems to give you more options for “insulating” your input data (not that you really need more options).

[Dean_C]

I use a function which I pass an array, it’s key is the incoming post/request vars value, and the value is it’s datatype.

e.g.

$data->cleanse(
        array(
                $_POST['somevar'] => INT,
                $_POST['anothervar'] => STR,
                $_POST['lala'] => STR_NO_HTML
        )
);

Inside of this method it does all the work. INT intval()'s the value, STR addslashes the value, STR_NO_HTML strips all html and then addslashes

[ceefour]

@Dean_C

I use a function which I pass an array, it’s key is the incoming post/request vars value, and the value is it’s datatype.

e.g.

$data->cleanse(
        array(
                $_POST['somevar'] => INT,
                $_POST['anothervar'] => STR,
                $_POST['lala'] => STR_NO_HTML
        )
);

Inside of this method it does all the work. INT intval()'s the value, STR addslashes the value, STR_NO_HTML strips all html and then addslashes

Seems like there are still lots of people using the [evil] addslashes().
Oh well… what do I care. It’s not like I’m a proper-escaping evangelist.

[Dean_C]

addslashes isn’t evil, it’s never failed me before now

Edit:

I have read your earlier posts, it’s interesting as I haven’t ever had that problem before but we’ll see

[stereofrog]

Well, all this ado about addslahes vs mysql_real_escape_string is pretty much about nothing. Mysql docs say black on white:

Strictly speaking, MySQL requires only that backslash and the quote character used to quote the string in the query be escaped. This function (mysql_real_escape_string) quotes the other characters to make them easier to read in log files.

addslashes escapes quotes and backslashes quite perfect, so why bother.

[RhythmAddict]

Easy. Use stored procedures with bind variables…

[Super_Phil]

@RhythmAddict

Easy. Use stored procedures with bind variables…

My prefered solution, although a lot of people here use mysql.

I like the bin2hex idea. It’s the simplest one so far but I haven’t seen how you get it back from hex. I think the main drawback is you can’t easily administer/report on data without being inside a script which can will reverse bin2hex(). Can anyone come up with a scenario where this technique could still be exploited by a cracker?
There’s also base64encode/decode. Could base64decode([insert SQL here]) ever produce a result which could exploit base64encode() ?
There is also pack()/unpack() which seems to give you more options for “insulating” your input data (not that you really need more options).

ugh - and lose the ability to search / order your data? And increase table size? The easiest solution is not the best.

[Young_Twig]

I will add (if it hasn’t already been said) that a nice (int) typecast is a quick way to avoid SQL injections in integer fields:

$userid=(int) $_POST['userid'];
$password=some_sql_injection_preventing_function($_POST['password']);

$query='SELECT * FROM users WHERE password\\''.$password.'\\' AND id='.$userid;

Now try the classic OR 1=1.

Userid: 5 OR 1=1
Password: ****

SELECT * FROM users WHERE password='asdf' AND id=5

[dbevfat]

As stated in posts above, this might have unwanted results, because:

1. values beginning with int, such as ‘12 or 25’, are casted to 12
2. values not beginning with an int, such as ‘blah blah’, are casted to 0

Both will result in valid queries, no error detected, but will at least return wrong data.

[system]

@dbevfat

As stated in posts above, this might have unwanted results, because:

1. values beginning with int, such as ‘12 or 25’, are casted to 12

But that’s a programming decision. The only values that should be casted to an int type are values that are ONLY allowed to be of an int type. “12 or 25” is a string value and should be escaped and quoted.

–ed

[Young_Twig]

@coo_t2

But that’s a programming decision. The only values that should be casted to an int type are values that are ONLY allowed to be of an int type. “12 or 25” is a string value and should be escaped and quoted.

–ed

Exactly. Like you wouldn’t typecast (int) on a search term.

[ceefour]

@Young_Twig

Exactly. Like you wouldn’t typecast (int) on a search term.

Do we really have to state something this obvious?

[Young_Twig]

@ceefour

Do we really have to state something this obvious?

:agree:

[Dylan_B]

@tezza

I like the bin2hex idea. It’s the simplest one so far but I haven’t seen how you get it back from hex.

<?php
function hex2bin($data)
{
   $len = strlen($data);
   return pack("H" . $len, $data);
}
?>

[mx2k]

does anyone have any methods that do not use sp (stored procedures) to prevent sql injection when dealing with php and mssql besides add slashes?

[dbevfat]

@coo_t2

But that’s a programming decision. The only values that should be casted to an int type are values that are ONLY allowed to be of an int type. “12 or 25” is a string value and should be escaped and quoted.

–ed

Well, that is obvious, isn’t it?

But if a certain value is supposed to come in, and is expected to hold an int, you’re going to cast it, right? And if someone send string “12 or 25” instead of an int, you’re obviosly casting a string to int.

[samsm]

There is so much crossfire in this thread, I have doubts that anything in it can be considered obvious.

[Roger_Ramjet]

@samsm

There is so much crossfire in this thread, I have doubts that anything in it can be considered obvious.

Agreed. :agree: And so much pedantic point-scoring that I doubt it will be much use to anyone anymore. Best thing everyone can do is to stand back and review what has already been said before adding to the confusion. This is afterall a VERY IMPORTANT issue of concern to us all.

[system]

@dbevfat

Well, that is obvious, isn’t it?

But if a certain value is supposed to come in, and is expected to hold an int, you’re going to cast it, right? And if someone send string “12 or 25” instead of an int, you’re obviosly casting a string to int.

My personal opinion is that the DB code/queries shouldn’t care what the user
input was, or how it was sent, etc.

@KTottE

Originally Posted by KTottE
SQL injection is a result of not validating input and/or not doing runtime type checks.

@stereofrog

I have to disargee. Injection problem has absolutely nothing to do with validation. That is, ‘valid’ data can cause an injection, and vice versa.

I agree with sterefrog. Input validation and protecting your DB from
SQL injection are two different things. Or they should at least be looked
at as two different things.

I separate my DB code into DAOs from the rest of the app so that validating
input from the _REQUEST variables isn’t even an issue. I don’t see
it as the DB layer’s responsibility.

Here’s a partial example of one of my DAOs:

   
   <?php
   
   class CampaignsDao_Mysql extends CampaignsDao {
   
   	
   	function CampaignsDao_Mysql($dbConn)
   	{
   		parent::CampaignsDao($dbConn);
   	}
   
   
   	function addCampaign( $name, $groupID, $pay_per_x_visits, 
 						 $cost_per_x_visits, $redirect_url=null )
   	{
   		if (!strlen($name) )
   		{   
 			$msg = 'Must provide campaign name as first parameter to '.		 'CampaignsDao_Mysql::addCampaign.';
   
 			return new DE_DataResourceError($msg, __LINE__, __FILE__);
   		}
   		elseif (!strlen($groupID))
   		{
   			$msg = 'Must provide a group ID as second '.
 				 'parameter to CampaignsDao_Mysql::addCampaign.';
   
 			return new DE_DataResourceError($msg, __LINE__, __FILE__);
   		}
   		elseif (!is_int($pay_per_x_visits) )
   		{   
   			$msg = 'Must provide an int value as third '.
 				 'parameter to CampaignsDao_Mysql::addCampaign.';
   
 			return new DE_DataResourceError($msg, __LINE__, __FILE__);
   		}
   		elseif (!is_numeric($cost_per_x_visits) )
   		{   
   			$msg = 'Must provide a numeric value as fourth '.
 				 'parameter to CampaignsDao_Mysql::addCampaign.';
   
 			return new DE_DataResourceError($msg, __LINE__, __FILE__);
   		}
   
   		// It doesn't necessarily have to be passed in as a float, 
   		// but the DB stores it as a float.
   		$cost_per_x_visits = (float)$cost_per_x_visits;
   
   		$name = mysql_real_escape_string($name, $this->_dbConn);
   
   		// $groupID is an alphanumeric string with length of 12
   		$groupID = mysql_real_escape_string($groupID, $this->_dbConn);
   	
   		if ($redirect_url !== null)
   		{
   			$redirect_url = 
 			 mysql_real_escape_string($redirect_url, $this->_dbConn);
   		}
   
   		$campaignsTable = $this->_campaignsTable;
   		
   		$campaignID = parent::_get_rand_id(12);
   
   		$query = "
   			INSERT INTO $campaignsTable 
   			VALUES ('$campaignID', '$name', $pay_per_x_visits,
 				 $cost_per_x_visits, '$redirect_url') ";
   
   		if (!$result = mysql_query($query, $this->_dbConn) )
   		{   
   			$msg = 'Error adding campaign in '.
 				 'CampaignsDao_Mysql::addCampaign : '.
   				    mysql_error();
   
 			return new DE_DataResourceError($msg, __LINE__, __FILE__);
   		}
   
   		$joinTable = $this->_campaignsGroupsJoinTable;
   
   		$query2 = "
   			INSERT INTO $joinTable 
   			VALUES ('$groupID', '$campaignID') ";
   
   		if (!$result2 = mysql_query($query2, $this->_dbConn) )
   		{   
   			$msg = 'Error adding campaign in '.
 				 'CampaignsDao_Mysql::addCampaign : '.
   				    mysql_error();
   
 			return new DE_DataResourceError($msg, __LINE__, __FILE__);
   		}
   
   		return $campaignID;
   	}
   }
   
   ?>
   
   

Now I do some validation here, but really, the data should of been checked
before the method call was made. Just like any other method/function call, you have to make sure you feed it the values it wants.
I don’t really see it as the DB layer’s job
to validate data from the HTTP request. In terms of validation, the DB layer
only really needs to do what’s necessary to do its job and protect itself.
It shouldn’t really care what the application considers to be “valid” data. So if “12 or 25” is passed in when it is
supposed to be an int value, it shouldn’t have even made it
to the point in your code that constructs a query string. So I guess my
point is, that input validation and SQL injection protection are two
different issues(as Stereofrog suggested earlier, you shouldn’t confuse them) that should not be mixed together. Once you start looking
at them as the same issue, then you’re data storage layer is having
too much influence over what your application code looks like.

–ed