Why mysql_real_escape_string() did not prevent hack?

Question

I've a website that hacked today. Server logs returned something like this as hacker's tries:

www.site.com/notifications.php?PID=7&id=999999.9%20union%20all%20select%20%28select%20distinct%20concat%280x7e%2C0x27%2Cunhex%28Hex%28cast%28schema_name%20as%20char%29%29%29%2C0x27%2C0x7e%29%20from%20%60information_schema%60.schemata%20limit%201%2C1%29%2C0x31303235343830303536%2C0x31303235343830303536%2C0x31303235343830303536--

But I've used mysql_real_escape_string() in my code:

if (isset($_GET['id']) && $_GET['id'] != '') {
   $id = mysql_real_escape_string($_GET['id']); 
} else {
   $id = '';
}

if ($id == '') {
   $stmt = "SELECT * FROM tbln13 ORDER BY id DESC"; 
} else {
   $stmt = "SELECT * FROM tbln13 WHERE id = $id";
}

$NewsResult = mysql_query($stmt) or die (mysql_error());

Why my website could not prevent this attack?

This is common sense. You're using deprecated functions, no prepared statements. All that's needed is [bobby tables](http://xkcd.com/327/) — Kermit, Dec 27 '13 at 19:29
`WHERE id = $id";`.... not quoted, so you're not treating it as a string; yet you're still escaping it as though it was a string — Mark Baker, Dec 27 '13 at 19:30
mysql_real_escape_string is for, as the name says, escaping special characters in a string, it will not make integers safe — Danijel, Dec 27 '13 at 19:38
This is a wake-up call to anyone still using the awful `mysql_query` interface. Throw that code in the garbage before it burns you again and write it properly using PDO and placeholders. — tadman, Dec 27 '13 at 19:51
You have other questions you posted that's related to PDO; why didn't you stick with that instead of using `mysql_*` functions? THAT, is what I don't get. It was only a matter of time till this happened. [This, being one of those questions](http://stackoverflow.com/questions/18133431/how-to-find-last-inserted-id-while-using-pdo-transaction) --- Now, you really need to read this post carefully => http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php and this one => https://www.owasp.org/index.php/Top_10_2013-Top_10 and STOP using `mysql_*` functions, once and for all. — Funk Forty Niner, Dec 27 '13 at 22:00
@FreshPrinceOfSO , because this problem belongs to one of my first projects — Mohammad Saberi, Dec 29 '13 at 20:53
@MohammadBagherSaberi The costliest expense is maintenance. Follow best practices the first time. — Kermit, Dec 29 '13 at 20:59

score 9 · Accepted Answer · answered Dec 27 '13 at 19:30

Because escape_string add slashes and such to quotes. You didn't have any quotes in your query, or the string they submitted.

Your query doesn't have a STRING in it, it appears to expect an int. If you expected an integer, you should have verified it was an int, or forced it to an int, before using it in a query. Escaping a value as a string, then using it as an int, won't work.

Switch to prepared statements in MySQLi or PDO.

Tomas · Answer 2 · 2013-12-27T20:09:23.523

The sql injected query looks like this

SELECT * FROM tbln13 WHERE id = 999999.9 
union all select 
 (select distinct concat(0x7e,0x27,unhex(Hex(cast(schema_name as char))),0x27,0x7e) 
  from `information_schema`.schemata 
  limit 1,1), 
 0x31303235343830303536, 0x31303235343830303536, 0x31303235343830303536--

as you see, you were injected because you have just allowed this!

You expected a number but you didn't check for it! So you got the number and something more.

You should have checked the $id variable for what you expected, which is the number. This is what I would use:

if (!preg_match('/^\d+$/', $id))
    die("ERROR: invalid id"); // error, don't continue

why the downvote? The explanation is not clear? This method is 100% correct. — Tomas, Dec 28 '13 at 12:15

score 1 · Answer 3 · answered Dec 27 '13 at 19:32

Use prepared statements, that will, in most cases, prevent SQL injections.

A simple and comprehensible guide to prepared statements can be found in this website:

Bobby Tables

More over you should stop using MYSQL, it's outdated and will be removed in future implementations. Use MySQLi or PDO instead.

score 1 · Answer 4 · answered Dec 27 '13 at 19:34

1

Because your escaped variable is not a string therefore it is not inside quotes in your query. If you want a quick fix you can change your query to:

$stmt = "SELECT * FROM tbln13 WHERE id = '$id'";

It is not standard use for numeric comparison but should work.

answered Dec 27 '13 at 19:34

Madcoe

213
1
6

A "quick fix" is what created this problem in the first place. The last thing that's needed is another one. – tadman Dec 27 '13 at 19:52
Sometimes fixes like this can save the day. Especially on PHP since it is not a strongly typed language... – Madcoe Jan 31 '14 at 05:39
Do not take chances with SQL queries. A single unescaped parameter could destroy your company or career. Quick fixes do not belong anywhere near this kind of code. – tadman Jan 31 '14 at 15:55

Mike Brant · Answer 5 · 2013-12-27T19:57:13.390

As mentioned by others, you should ditch deprecated mysql_* functions and instead used prepared statements via mysqli or PDO.

Even then you should also be validating your input, because just using prepared statements will not help you identify whether you have input values that are valid. You would ideally make sure all your input is valid before even attempting a prepared statement. In this case, this validation could be as simple as this:

$id = filter_input(INPUT_GET, 'id', FILTER_VALIDATE_INT);
if (false === $id) {
   // you do not have a valid integer value passed. Do something.
} else {
   // continue with your prepared statement
}

Why mysql_real_escape_string() did not prevent hack?

5 Answers5

Linked