SQL Injection and mysql_real_escape_string

Durux · 05-01-2008, 10:49 AM

I have heard that it is still possible to use SQL Injections even with the use of mysql_real_escape_string() though not the most common ones.

If this is true, why not use a preprepared SQL sentence.
This will be a slower code of course, but it will eliminate SQL Injections in a login form.

EX:

PHP Code:


			
/*Inputted: $pUsername and $pPassword from the login form*/



$link = mysql_connect('localhost','username','password') or die('Invalid connection: '.mysql_error());

$db = mysql_select_db('database',$link) or die('Invalid Database: '.mysql_error());



$sql = "SELELCT username, password, rank FROM example_users";

$result = mysql_query($sql,$link) or die('Invalid Query: '.mysql_error());



while($row = mysql_fetch_assoc($result)){

    

    $szArray[]['username'] = $row['username'];

    $szArray[]['password'] = $row['password'];

    $szArray[]['rank'] = $row['rank'];

    

}



$j = count($szArray);

$salt = 'not this one';/*You just never know*/

$pPassword = md5($salt.$pPassword);

$chBool = false;



for($i=0; $i<$j; $i++){

    

    if($szArray[$i]['username'] == $pUsername){

        

        if($szArray[$i]['password'] == $pPassword){

            

            $chBool = true;

            $user = $szArray[$i]['username'];

            $i = $j;

            

        }

        

    }

    

}



mysql_close($link);



if($chBool == true){

    echo "Welcome $user.";

}

else{

    echo "Wrong username or password.";

}

With this there is no way you could use SQL Injection.
I know this can not be made for every single Query but for the most important like the login it could be quite good.

This of course only goes if it's true that even mysql_real_escape_String() can't stop every SQL Injection.

delayedinsanity · 05-01-2008, 02:14 PM

I've read a lot on the various things that mysql_real_escape_string() catches that addslashes() doesn't, but I haven't come across anything specifying what it doesn't catch. That's not to say it doesn't, but it really shouldn't be your only line of defense anyways. Validate the data you're receiving before you use it! If somebody is entering a username into your login form, there's no reason they should be using special characters such as ' or - anyways. ~[A-Za-z0-9_]~ stops that dead before it even gets so far as being used in an SQL query, for example.

When you said prepared statements at first, I thought you were referring to MySQL :: Prepared Statements which is something I was inquiring about in another thread. Personally, I couldn't image doing what you suggest there, what if you had 20,000 users??
-m

freenity · 05-01-2008, 02:20 PM

I haven't tried it, but I think you can make a sql injection even using mysql_real_escape_string(). (I don't know if mysql_real_escape_string() removes spaces or not)
Check this:

"SELECT * FROM user WHERE id = $id"

What id $id has this?
$id = "99999 OR id > 0";

This should list all the users. The problem there is that $id is not between ', so the attacker don't have to write '. But the problem are the spaces, if they are removed everything will be fine.

Village Idiot · 05-01-2008, 02:35 PM

Valid injection is an issue, wildhoney uses sprintf to secure it form that. I find the best way it to wrap all values in single quotes and table names, ect. in `. For instance
"SELECT * FROM user WHERE id = $id"
Would be
"SELECT * FROM `user` WHERE `id` = '$id'"

That way, even if $id = "99999 OR id > 0"; it wont inject anything.

Dont use stripslashes, there are valid uses for slashes and that would simply remove them all.

freenity · 05-01-2008, 02:38 PM

yes, wrapping the variables in single quotes wil work fine, but you compare a number with a string, and it works on mysql, it could fail on other sql database.

Village Idiot · 05-01-2008, 02:42 PM

Quote:

Originally Posted by freenity

yes, wrapping the variables in single quotes wil work fine, but you compare a number with a string, and it works on mysql, it could fail on other sql database.

Then its a good thing we aren't talking about other databases. That would be one of the multiple things you would have to change to work with Oracle, MSSQL, ect.

delayedinsanity · 05-01-2008, 02:44 PM

You can also circumvent the basic sql injections where somebody tries

' OR username=admin --

simply with trim($szUsername, " '-"). Or preg_match("~[A-Za-z0-9_]~", $szString), or... as I said above, the best way, imo, is to not trust just one method of sanitization or validation. Verify that information.
-m

Village Idiot · 05-01-2008, 02:46 PM

Quote:

Originally Posted by delayedinsanity

You can also circumvent the basic sql injections where somebody tries

' OR username=admin --

simply with trim($szUsername, " '-"). Or preg_match("~[A-Za-z0-9_]~", $szString), or... as I said above, the best way, imo, is to not trust just one method of sanitization or validation. Verify that information.
-m

There are sometimes valid uses for those letters. You don't want a security method that hurts usability.

delayedinsanity · 05-01-2008, 02:48 PM

And in those instances, you use other methods. I wasn't suggesting that would clear up all the problems, the primary point of that was to say, don't trust one method. I sound like a broken record though, so I'm going to let it go after this. However, I will say, I can't think of a single instance where ' needs to be on the outside of a user inputted string.
-m

Village Idiot · 05-01-2008, 02:52 PM

Quote:

Originally Posted by delayedinsanity

And in those instances, you use other methods. I wasn't suggesting that would clear up all the problems, the primary point of that was to say, don't trust one method. I sound like a broken record though, so I'm going to let it go after this. However, I will say, I can't think of a single instance where ' needs to be on the outside of a user inputted string.
-m

I cant either, but one could come up. When it does months later, you will go around for a long time trying to figure out what is wrong with what you just wrote. It wont immediately hit you that the injection protection did it.

As you shouldnt trust one method, just do what the manual says (the mysql manual says use mysql_real_escape_string() and quotes). Unless you can say you know mysql better than the people who made it and test its security, dont make your own method. It will either be insecure or unnecessary. I know every time I try to make my own method for something like this, it ends up bad.

delayedinsanity · 05-01-2008, 03:34 PM

Quote:

Originally Posted by Village_Idiot

Unless you can say you know mysql better than the people who made it and test its security, dont make your own method.

I think you misunderstood me. I wasn't suggesting recreation of the wheel. I use mysql_real_escape_string. What I was referring to was not relying on mysql_escape_string to save your life.
-m

wGEric · 05-01-2008, 08:37 PM

Quote:

Originally Posted by freenity

I haven't tried it, but I think you can make a sql injection even using mysql_real_escape_string(). (I don't know if mysql_real_escape_string() removes spaces or not)
Check this:

"SELECT * FROM user WHERE id = $id"

What id $id has this?
$id = "99999 OR id > 0";

This should list all the users. The problem there is that $id is not between ', so the attacker don't have to write '. But the problem are the spaces, if they are removed everything will be fine.

Since it is an integer, cast it as one. Don't try to use it as a string.

PHP Code:


			
$id = intval("99999 OR id > 0");

Problem solved.

freenity · 05-01-2008, 08:39 PM

Quote:

Originally Posted by wGEric

Since it is an integer, cast it as one. Don't try to use it as a string.

PHP Code:


			
$id = intval("99999 OR id > 0");

Problem solved.

nice :D

Also can be:

PHP Code:


			
$id = (int)$id;

Village Idiot · 05-01-2008, 10:18 PM

Quote:

Originally Posted by wGEric

Since it is an integer, cast it as one. Don't try to use it as a string.

PHP Code:


			
$id = intval("99999 OR id > 0");

Problem solved.

Typecasting takes longer. There is no downside to putting quotes around the value in the query. Typecasting also does not take care of string values, which you will have to use quotes with. Typecasting will do the job, but it leaves a greater possibility of forgetting to do it. If you just type everything in with a quote, there is a far less chance of forgetting.

Highway of Life · 05-02-2008, 05:41 PM

Type casting is extremely important in any programming language, and PHP is no exception.

Although type casting is an often-missed tool when a PHP developer is trying to ensure data integrity.
Type casting is most often used to specifically enforce a type in order to provide extra security or just to make sure a set type of data is being used. For example, if your script absolutely requires an integer number, it's a smart move to typecast your variable with (integer) so that PHP will convert any other type to integer or do nothing if the type is already integer.

All user data being input in an SQL Query must be sanitised in one way or another, typecasting is one of the most important ways of ensuring that your integers are integers, your floats are floats, bool, binary, array or strings. -- or to ensure that your variable is an object or resource.
In many cases, type casting is sufficient validation. When a variable needs to be an integer, you won’t need to spend additional time and server resources sanitising the variable as you would with a string.

Suggesting that type casting is not important displays a lack of programming knowledge and especially security awareness.
Dealing with database interaction is something to always take seriously — and type casting can be of great benefit to you.

Village Idiot · 05-02-2008, 06:36 PM

Quote:

Originally Posted by Highway of Life

Suggesting that type casting is not important displays a lack of programming knowledge and especially security awareness.
Dealing with database interaction is something to always take seriously — and type casting can be of great benefit to you.

You gave a number of claims with nothing to support what you are saying. You also gave no downside to not typecasting besides it being no good (which is a matter of opinion).

You have to clean everything one way or another, why take an extra step and typecast when it will already be secure. It is not a big deal if you check if your primary ID is "a". If you typecast, it will be equal to nothing (""). One way or another you will get an empty set returned. There is no greater security risk in either method when compared to the other.

I am not saying type casting is the wrong way, or that it wont get the job done. It simply isn't necessary if you use other security methods. But to say not typecasting is showing lack of knowledge is nothing short of ignorant.

Highway of Life · 05-02-2008, 08:05 PM

Claims and or examples are not necessary to demonstrate basic and correct programming concepts.

That said, I’ll give a very basic example of proper usage and why type casting it of utmost importance when programming.
If you are interacting with the database, MySQL query for example, it’s important to use typecasting in the following example:

PHP Code:


			
$sql = 'SELECT field1, field2, field3
        FROM some_table
        WHERE field_name = ' . (int) $var_id;

$var_id cannot be a string otherwise you open it up to SQL Injection.
If you sanitise the variable with mysql_escape_string(), you are wasting resources... instead, you should force var type integer.
Enclosing the variable into single quotes and believing that it is somehow secure again gives a false sense of security and leaves the query open once again to SQL Injection.
Any string inserted into an SQL Query must be enclosed in single quotes -- along with mysql_escape_string() for the user-input, but integers should not, as demonstrated in the example above.

In the case of SQL UPDATE or INPUT, if you do not force var type int, you will also have SQL Errors if the value is not strictly an integer.

You don’t use shortcuts when security is in question -- you do it right.
All basic programming concepts with any language, especially PHP.
Anyone who has worked in C or Java knows how important typecasting is.

Village Idiot · 05-02-2008, 08:14 PM

Quote:

Originally Posted by Highway of Life

Claims and or examples are not necessary to demonstrate basic and correct programming concepts.

That said, I’ll give a very basic example of proper usage and why type casting it of utmost importance when programming.
If you are interacting with the database, MySQL query for example, it’s important to use typecasting in the following example:

PHP Code:


			
$sql = 'SELECT field1, field2, field3
        FROM some_table
        WHERE field_name = ' . $var_id;

$var_id cannot be a string otherwise you open it up to SQL Injection.
If you sanitise the variable with mysql_escape_string(), you are wasting resources... instead, you should force var type integer.[/php]

Your argument for typecasting is based off of speed, not security as you earlier claimed. It will hardly take any more time to clean via mysql_real_escape_string() than to typecast. The difference will be negligible.

Quote:

Originally Posted by Highway of Life

In the case of SQL UPDATE or INPUT, if you do not force var type int, you will also have SQL Errors if the value is not strictly an integer.

You should be validating a lot more then just type when using those commands. However, not typecasting leaves room for an error, not a hole.

Quote:

Originally Posted by Highway of Life

You don’t use shortcuts when security is in question -- you do it right.
All basic programming concepts with any language, especially PHP.

How is my method more of a shortcut than yours? I could just as easily call your method a shortcut not to use mysql_real_escape_string all the time. Do your security right.

Quote:

Originally Posted by Highway of Life

Anyone who has worked in C or Java knows how important typecasting is.

You don't have a choice in C or Java to typecast or not, it can be the biggest pain in the ass sometimes. I know C++, although I wont claim to be more then intermediate.

Your arguments for typecasting are not based off of security. Please dont come here and present personal style as fact.

Village Idiot · 05-02-2008, 08:20 PM

I should also add that in my cleaning function, I don't use mysql_real_escape_string() if its an integer, but that is on a function basis; not inline.

wGEric · 05-02-2008, 09:45 PM

Quote:

Typecasting takes longer.

So you are saying performance is more important than security?

Quote:

There is no downside to putting quotes around the value in the query.

PHP Code:


			
'1' !== 1

Which is what you are trying to do unless all of your columns are some sort of string type. That's the downside. You are trying to compare a string with an integer or you're inserting a string into an integer column. That doesn't make sense.

Quote:

Typecasting also does not take care of string values, which you will have to use quotes with.

That's what mysql_real_escape_string and other methods for cleaning strings are for. They aren't for numbers. That's why it has string in the name.

Quote:

I should also add that in my cleaning function, I don't use mysql_real_escape_string() if its an integer, but that is on a function basis; not inline.

Wait, so you are using a string that hasn't been sanitized? Or do you type cast it as an integer before using it? If the later, why bother making it a string in the query. You already know it is secure.

By not typecasting you aren't forcing a variable to be what you expect and want it to be. If you want an integer, make it an integer so you aren't dealing with strings which can be exploited. It's so much easier to secure an integer than a string.