[RESOLVED] SQL Injection

loll

I have been reading up on SQL Injection and I am trying to secure my scripts to prevent this. However, I am coming up against a brick wall in regards to this.

I understand the use of the mysql_real_escape_string() command to help with this, however this isnt helping me with a "union" type injection. as there are no special characters in the appended SQL statement.

So Im wondering what the best way to prevent this sort if injection is.

If the data is just a number then its quite simple, jsut remove any on numeric data before running the query, but in the case of other types where the data could be any character (like a message board message entry screen!) then such validation would not work.

so my understanding is that my code should look like this:

include(dbconnect.php);
$message = $_POST['message'];
$message = mysql_real_escape_string($message);

at this point, it should be safe against other sorts of attacks (as far as I am aware, please correct me if this is wrong)

But its not protecting me against the union attack, and I have no idea where to start on that.

I could just not allow the word union be used, but that wouldnt be a great idea, so im wondering how I do prevent this sort of attack,

Any help would be greatly appreciated.

NogDog

A UNION would only be a danger if it were inserted into your query outside of a (quoted) string literal. Since the only user-supplied values you should be allowing into your queries in an unquoted manner are numeric values, that should not be an issue if you are escaping string values.

In other words, this query is safe:

SELECT * FROM `table` WHERE `col` LIKE '%UNION SELECT * FROM USERS%'

As far as the SQL interpreter is concerned, the "union" stuff there is just part of the string literal and will not be processed as an actual UNION query. So if you ensure that any numeric values you use in the query are numeric and that any strings you use are escaped and then correctly quoted, you should be OK.

And of course you could move up to the MySQLi extension (or PDO) and use prepared statements, letting PHP/MySQL do the type-checking and escaping for you.

loll

Just so I understand what you are saying:

the following query would be vunerable based on the fact that the variable isnt quoted:

select * from table where id=$id;

where as the following query would be safe from sql injection

select * from table where name='$name';

so if this is correct, does that mean that in general, all queries should be run with the variables wrapped in mysql_real_escape_string() but if the variable is unquoted, it should also be stripped form it so it could look something like this

for numerical variables

$id = eregi_replace("^[0-9]","",$_POST[id]);
$query = "select * from table where id=" . mysql_real_escape_string($id);
$result = mysql_query($query);

and for string variables

$query = "select * from table where name='" . mysql_real_escape_string($name) . "'";

is this correct?

Thank you for your help!

NogDog

Generally, I would recommend rejecting invalid inputs rather than just cleaning them (perhaps other than trim()-ing them). But regardless of that usability decision, one way to approach it is to use [man]sprintf/man to build the query, using the appropriate place-holder for the user-supplied values. To some degree this emulates the use of place-holders in prepared statements.

$sql = sprintf(
   "SELECT * FROM `table` WHERE `id` = &#37;d AND `name` = '%s'",
   $_POST['id'], // %d will convert to integer
   mysql_real_escape_string($_POST['name']) // %s will treat as normal string
);
$result = mysql_query($sql);

If you are the sort who likes to wear a belt and suspenders, you could also cast the numeric value to the desired type:

$sql = sprintf(
   "SELECT * FROM `table` WHERE `id` = %d AND `name` = '%s'",
   (int) $_POST['id'], // make absolutely sure it's an integer!
   mysql_real_escape_string($_POST['name']) 
);
$result = mysql_query($sql);

laserlight

NogDog wrote:
If you are the sort who likes to wear a belt and suspenders, you could also cast the numeric value to the desired type

Though that would not be very useful since the %d format specifier effectively performs such a conversion. (But then you already noted this, strangely.)

NogDog

laserlight;10925074 wrote:
Though that would not be very useful since the %d format specifier effectively performs such a conversion. (But then you already noted this, strangely.)

It was sort of an attempt at geek humor, I guess. :rolleyes: It's based on something I read somewhere discussing an ancient Asian cavalry unit where the soldiers were armed with bows and arrows, some sort of pole arm, and some sort of side arm (sword or mace, or maybe both?). The author suggested that this sort of soldier was comparable to a man who "wears a belt and braces* and also carries a ball of string in his pocket, just in case." 🙂

"braces" = "suspenders" on this side of the Atlantic

loll

thank you for your help, it s greatly appreciated, I think I will leave the suspenders in the depths of the closet, and go with method one 😃

It had never occurred to me to use sprintf

Thank you!