[RESOLVED] SQL Injection Prevention

[pages]

Hi didnt know whether to post this on the Coding section since it will contain code but here goes !

I am currently finishing off a project and as i was creating this project I became more and more aware of the dangers of SQL Injection but since i didnt fully understand how to fully prevent it I left the issue for "do it when im finished"
Well now i am finished its come time to do it.

From my basic understanding , the only way someone can "hack" your website without being in the database itself is through
1) the use of textboxes
2) wherever $_request wants something from the URL

I presume any information entered through the use of radio buttons/checkbox/drop down is safe since its pre-defined by the website ? might be wrong ??

Now what im wondering is , is there a php function which can eliminate or severely reduce the risk of SQL injection ?

At first i used

function clean($str) {
		$str = @trim($str);
		if(get_magic_quotes_gpc()) {
			$str = stripslashes($str);
		}
		return mysql_real_escape_string($str);

But to my surprise i was told this type of function would actually help SQL injection cause it would remove in-built protection.

I have been looking around the net for snippets of code and found things like

If (!get_magic_quotes_gpc())
		{
		$searchterm = addslashes($searchterm);
		}

But again im not really sure this would actually help.

Basically what im asking for is advice on how to stop SQL injection where the only 'leak' can come from string varibles in textboxs and again strings from URL .

If you can help at all it would be much appreciated.

[laserlight]

pages wrote:

From my basic understanding , the only way someone can "hack" your website without being in the database itself is through
1) the use of textboxes
2) wherever $_request wants something from the URL

I presume any information entered through the use of radio buttons/checkbox/drop down is safe since its pre-defined by the website ? might be wrong ??

You would be wrong. You should assume that an attacker can exploit any incoming data, whether it be from a textbox in a form sent by the post method, or a hidden field in the same form, the value of a radio button, checkbox or drop down menu, or by the query string, or even from a cookie value.

pages wrote:

Now what im wondering is , is there a php function which can eliminate or severely reduce the risk of SQL injection ? (...) But to my surprise i was told this type of function would actually help SQL injection cause it would remove in-built protection.

Those who advised you thus are mistaken. The "in-built protection" provided by magic_quotes_gpc is flawed, and nowadays disabled by default. When dealing with string data with the MySQL extension, mysql_real_escape_string() is appropriate.

It would be better to switch to the MySQLi or PDO extension and use prepared statements. Prepared statements separate the data from the SQL statement, thus reducing the possibility of SQL injection even further.

[NogDog]

[EDIT]LOL...there seems to be a slightly distorted echo here. 🙂 [/EDIT]

Any form data is potentially suspect, as a hacker does not have to use the exact form on your web page: s/he can use cURL and similar mechanisms to submit his/her own custom HTTP request with whatever data is desired for any form field.

As long as you use the correct escaping mechanism for the DBMS being used, whether that be a function such as mysql_real_escape_string() or prepared statement place-holders as in the MySQLi or PDO interfaces, then you will prevent SQL injection. (This does not necessarily prevent other modes of attack, but it does take care of SQL injection.)

I do not recommend addslashes(), as it is generic and not DBMS-specific, whereas mysql_real_escape_string() is (assuming you're using the original MySQL interface). If you do not have control over the server such that you can ensure magic_quotes_gpc is turned off (or you want to create a generally available app that can work in multiple environments), then the stripslashes() may be needed to undo the magic_quotes_gpc "damage" if it is in effect. Your first function is therefore fine, assuming you are using the standard MySQL interface: you are undoing the magic_quotes if they are in effect, then applying the appropriate escaping mechanism for your DBMS. Whoever told you it was bad was incorrect.

However, you might want to look into using the newer MySQLi (or PDO) extension and make use of prepared statements, which automate the escaping for you (though you still may need to undo magic_quotes damage if you cannot control the PHP settings).

[pages]

jeez , if im being honest i didn't even know there was such a thing as MySQLi . Just downloaded Xampp assuming it had to best/most up-to-date MySQL/apache.

Hmm the only problem i now face is the due in date is fast approaching and i still need to write about 20,000 to 30,000 word report on the Project. 🙁 (current word count = 0 )

In your experience would i be better using some form of them functions to protect or switching to MySQLi ?
And if you suggest the functions have you any ones which you yourself use that you could show ?

[Weedpacket]

pages wrote:

assuming it had to best/most up-to-date MySQL/apache.

[man]MySQLi[/man] is a PHP extension, not a different version of MySQL.

[NogDog]

pages;10949524 wrote:

jeez , if im being honest i didn't even know there was such a thing as MySQLi . Just downloaded Xampp assuming it had to best/most up-to-date MySQL/apache.

Hmm the only problem i now face is the due in date is fast approaching and i still need to write about 20,000 to 30,000 word report on the Project. 🙁 (current word count = 0 )

In your experience would i be better using some form of them functions to protect or switching to MySQLi ?
And if you suggest the functions have you any ones which you yourself use that you could show ?

I have used a function nearly identical to the clean() function in your original post, and it worked just fine for me. These days I'm more likely to use the MySQLi extension and make use of prepared statements, but it also has a mysqli_real_escape_string() method you can use for non-prepared-statement usage in much the same way.

Currently, probably 90% of the time I use the Active Record class in the CodeIgniter framework and let it deal with these things without direct intervention by me. 🙂

[pages]

Hmm well cheers for the reply
I've set myself a deadline to completely finish all bits of coding basically in 15 hours time (8 of which will be sleep ) in order to start write up , else i doubt id be able to write near 30,000 words

Just dont think id be able to get the extension and change all the pieces of code in time without leaving some serious mistakes , from lack of proper testing.

Think ill just try and use mysql_real_escape_string() type things around my $post and $request stuff and hope to god that satisfies even a poor man's version of Injection Protection.

Would nearly kill at this stage for a one for all function which takes in a string and makes it safe 🙂

[bradgrafelman]

Just to give another example of how to sanitize data, say you have a user id coming from a cookie - $_COOKIE['id']. Since cookies are user-supplied data, it can easily be altered into something malicious and thus needs to be sanitized. Now, if your code looked like:

$id = mysql_real_escape_string($_COOKIE['id']);
$query = "SELECT foo FROM users WHERE id = $id";

you'd still be vulnerable to SQL injections. If I sent 2 OR 1 = 1, the query would end up looking like:

SELECT foo FROM useres WHERE id = 2 OR 1 = 1

and my SQL injection will be successful.

Why? Because the above code uses an incorrect function - numeric data isn't a string, thus [man]mysql_real_escape_string/man isn't appropriate. Instead, since I define ID's as being integers, I might do something like:

$id = (int)$_COOKIE['id'];

instead.

[NogDog]

Good point from Brad, and it reminds me of one thing I typically do in addition, which is to use sprintf() to build the query string, using applicable place-holders to force numeric types:

$query = sprintf(
   "SELECT * FROM `table` WHERE `id` = %d AND `name` = '%s'",
   clean($_POST['id']),
   clean($_POST['name'])
);
$result = mysql_query($query);

[pages]

Once again thanks for the replies

And yeah fortunately the only things which i know how to prevent injection in are int values for id's 🙂

so if i did end up using something like

function clean($str) { 
        $str = @trim($str); 
        if(get_magic_quotes_gpc()) { 
            $str = stripslashes($str); 
        } 
        return mysql_real_escape_string($str); 

Do you think this would be viewed as good enough protection ?? My main concern is an external examiner the first thing they do for the login is
'x'; DROP TABLE members; --'; -- Boom!

[bradgrafelman]

pages wrote:

Do you think this would be viewed as good enough protection ??

As long as you only use that function on strings and not numeric types, sure. I would also get rid of the '@' error suppressor as well. Also note that [man]get_magic_quotes_gpc/man isn't a valid function in PHP 6, so if you wanted your code to be a bit more forwards compatible, you could use [man]function_exists/man to make sure that it's a function before you use it.

[pages]

ok so

function clean($str) { 
        $str = trim($str); 
        if(functionexsists('get_magic_quotes_gpc')) { 
            $str = stripslashes($str); 
        } 
        return mysql_real_escape_string($str); 

Hmm wierd i never new trim could even throw up an error just had it there cause i was lazy

Sorry if its wrong but from php.net i think i got the syntax right !

but its late in ireland and this little leprechaun needs to be awake early to find his pot of gold 🙂
But once again thank you for all the help ! maybe one day in about 15 years when im good at php i might be able to help someone else out ....might !

[bradgrafelman]

pages wrote:

i think i got the syntax right

Not quite; you misspelled the function name and you omitted the actual call to the get_magic_quotes_gpc function. The if() statement should look something more like:

if(function_exists('get_magic_quotes_gpc') && get_magic_quotes_gpc()) {

[pages]

Thank you once again im in your debt !

[quark76]

Are you saying something like this:

NogDog;10949566 wrote:

Good point from Brad, and it reminds me of one thing I typically do in addition, which is to use sprintf() to build the query string, using applicable place-holders to force numeric types:

$query = sprintf(
   "SELECT * FROM `table` WHERE `id` = %d AND `name` = '%s'",
   clean($_POST['id']),
   clean($_POST['name'])
);
$result = mysql_query($query);

Prevents this:

SELECT foo FROM useres WHERE id = 2 OR 1 = 1

Or is it an alternative to this:

$id = (int)$_COOKIE['id']; 

[bradgrafelman]

@: Yes, that's essentially what using [man]sprintf/man with a numeric placeholder does.

Basically, the data '2 OR 1 = 1' doesn't contain any special SQL characters, so using [man]msyql_real_escape_string/man would be inappropriate and insufficient, since you aren't parsing string data (which is what that function is meant for).

Instead, using %d or %u in the sprintf() string automatically forces the variable to be an integer (signed or unsigned, respectively), so '2 OR 1 = 1' would get type-casted down to an int, namely 2.

[quark76]

bradgrafelman;10955925 wrote:

@: Yes, that's essentially what using [man]sprintf/man with a numeric placeholder does.

Basically, the data '2 OR 1 = 1' doesn't contain any special SQL characters, so using [man]msyql_real_escape_string/man would be inappropriate and insufficient, since you aren't parsing string data (which is what that function is meant for).

Instead, using %d or %u in the sprintf() string automatically forces the variable to be an integer (signed or unsigned, respectively), so '2 OR 1 = 1' would get type-casted down to an int, namely 2.

Ok...thanks!

Now with the same function the OP has developed, would you not need functions like strip_tags & htmlentities for any string data that was not an int only?

[bradgrafelman]

quark76 wrote:

would you not need functions like strip_tags & htmlentities for any string data that was not an int only?

Now you're venturing away from the topic of SQL security and more into XSS/HTML issues.

To answer your question, though, the answer is maybe and no.

Do you need those functions with INSERT'ing data? No. My opinion is to INSERT the data as it was given, in most cases.

Do you want those functions when displaying data? Maybe. It depends on the application.

[quark76]

bradgrafelman;10955935 wrote:

Now you're venturing away from the topic of SQL security and more into XSS/HTML issues.

To answer your question, though, the answer is maybe and no.

Do you need those functions with INSERT'ing data? No. My opinion is to INSERT the data as it was given, in most cases.

Do you want those functions when displaying data? Maybe. It depends on the application.

Ok. Got it, so don't worry about html being stored, because it has not impact on SQL security, just catch it on the way out if you don't want users using HTML. Nice, very helpful.

Thanks Brad.