1

I have the following code :

$req = mysql_query("SELECT * FROM table WHERE id='".mysql_real_escape_string($id)."' ORDER BY lastupdate DESC LIMIT ".mysql_real_escape_string($_GET['start']).", 15 ");

For some reason it seems like I can inject some (unusable) SQL code into the $_GET['start'] parameter.

For example : 1,100 # (url encoded like this : +1%2C+100%23)

I'm fairly sure you can't really use it to do any damage or steal anything from my db (UNION can't work because of ORDER BY, and mysql_query() doesn't allow multiple queries

I know I should add intval() to make sure it is an integer. My main question is WHY... Why does it work? I really don't understand.

Thank you very much for your insights.

Phlocon
  • 11
  • 2
  • http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string – adeneo Apr 05 '15 at 17:34
  • You can’t exploit this with a [`UNION`](http://dev.mysql.com/doc/refman/5.6/en/union.html) because in order to apply `ORDER BY` or `LIMIT` to an individual `SELECT`, you need to place the clause inside the parentheses that enclose the `SELECT`. However, [there are other clauses after the `LIMIT` clause that may be exploitable](http://stackoverflow.com/a/22887526/53114). – Gumbo Apr 05 '15 at 17:47
  • I read that over and over again and I still can't find out why... The only thing I'm thinking is that maybe it is a quote thing... But I can't do this : LIMIT '1',15 so I don't see a way to secure that... – Phlocon Apr 05 '15 at 17:49
  • You need to understand what `mysql_real_escape_string` actually does. Unfortunately, the [official manual page](http://php.net/mysql_real_escape_string) is not that specific on how to use it properly. The “Escapes special characters in a string for use in an SQL statement” should rather read: “Escapes special characters in a string for use in a **MySQL string literal**”. – Gumbo Apr 05 '15 at 17:51

1 Answers1

0

You should parse it into an integer.

$req = mysql_query("SELECT * FROM table WHERE id='".(int)$id."' ORDER BY lastupdate DESC LIMIT ".(int)$_GET['start'].", 15 ");

Additionally, you should strongly consider using mysqli or PDO

The mysql_* , mysqli_* functions have no idea of what the structure of your table is. The function mysql_real_escape_string considers everything as string, and just makes it safe.

Manu
  • 901
  • 1
  • 8
  • 28