Wrong tool for the job

This time I invite the dear reader to an investigation with a twist. It started as an SQL injection hunt, but it soon turned into experiencing the futility of existence, questioning the rules by which the universe operates, and finally, the reevaluation of the impossible.

Although the star of the show is not the SQL injection, this story - like many other stories - starts with a simple query.

$query = "SELECT * FROM users WHERE id={intval($id)}";
$user = $db->query($query)->fetch();

Depending on the project's coding conventions, this could totally slip through a code review. Especially if the reviewers are not that familiar with PHP. Maybe a half-hearted question would arise: Is that intval call really working? But the tests were green, so everybody got convinced. No problems there.

Or is it? Let's try this out...

$ php -a
php > $id = 'apple tree';
php > var_dump("{intval($id)}");
string(20) "{intval(apple tree)}"

It looks like that PHP's variable substitution is not as smart as other languages' similar constructs (for example, the template literals in Javascript or the literal string interpolation in Python).

The intval function is not executed, so we arrived at the possibility of SQL injection. For example, if the value of the $id variable would be something like 1)} OR 1=1 --, then... but that's not interesting anymore, right? For me, at least, the big question was at this point: Why were the tests green?

My first guess was that maybe MySQL also has an intval function, and perhaps that's the one that got executed, but after a quick check, I was convinced that it's not the case.

$ mysql
> SELECT intval("1234");
ERROR 1305 (42000): FUNCTION mysql.intval does not exist

It must be something with the curly brackets. After a long searching session, I finally found a promising clue in the MySQL documentation:

{identifier expr} is ODBC escape syntax and is accepted for ODBC compatibility.

I jumped into researching the "ODBC escape syntax" like a bloodhound that picked up the scent. I wish I had read the documentation further instead.

ODBC is a standard API that can be used to make our code independent from the type of database server we use. This contraption between the curly braces is meant to help with that so the concrete driver can transform specific expressions so that the database server can handle them.

We aren't using ODBC driver in PHP, so there was nothing that could preprocess these expressions for the MySQL server. So it remained as the only suspect. We should interrogate it to find out why it runs that query.

$ mysql
> SELECT {intval("1234")};
+------+
| 1234 |
+------+
| 1234 |
+------+
1 row in set (0.00 sec)

> SELECT {intval("asdf")};
+------+
| asdf |
+------+
| asdf |
+------+
1 row in set (0.00 sec)

> SELECT {hmmm(1234)};
+------+
| 1234 |
+------+
| 1234 |
+------+
1 row in set (0.00 sec)

> SELECT {erdekes 1234};
+------+
| 1234 |
+------+
| 1234 |
+------+
1 row in set (0.00 sec)

As you can see, between the curly braces, the intval seems to be working... except when you give it some text... or you don't call it intval either... maybe you forget the brackets as well.

As confirmed by the next sentence in the documentation:

The value is expr.

So the identifier could be anything. We will get back the given expression as the result. It won't do anything with it. Maybe MySQL only cares about it, so it won't fail if the ODBC driver leaves something like this in the query.

That solves our mystery. The only thing left to do is think about how we don't run into this trap in the future. For example, we could use prepared statements instead of intval for escaping. We could also have some tests that call our code with non-numeric ids to see if there's a proper escaping in place.

Ez a bejegyzés magyar nyelven is elérhető: Jó lesz az, csak másra

Have a comment?

Send an email to the blog at deadlime dot hu address, or visit the tweet related to this post.

Want to subscribe?

We have a good old fashioned RSS feed if you're into that, and you can also follow the blog on Twitter.