Code Yellow B.V. — What your framework never told you about SQL injection protection
A lire car révélateur de beaucoup de mauvaises pratiques (et ça fait peur) … une phrase à retenir :
The implementations in CakePHP, CodeIgniter and FuelPHP before the patch were the worst of all. They were so incredibly large and complex they couldn’t possibly be safe
We’ve discovered that SQL injection is to this day not a fully solved problem, even in most popular frameworks. In this post, we’ll explain how these frameworks fail at escaping parts of a query, culminating in the discovery of a critical vulnerability in the popular Laravel framework which affects a large percentage of applications.
Let’s start with an innocent example, which provides the starting point of our journey. This is a typical simple use case: a filterable, sortable list.
This would return products, optionally filtered by a minimum price. It uses the input directly, without checking whether the price is syntactically a valid number. Now, that’s pretty lazy, but regardless of not validating, this code is protected by the framework against SQL injection. If the input is non-numerical, and we’re using a sane database, this would cause a data constraint error, resulting in an exception being thrown in PHP. It can be argued that this is fine: there is nothing sane you can do with bogus input, so returning a HTTP status of 500 is acceptable. The main point is: this code is not vulnerable to SQL injection.
Again, you might argue that it’s a bad idea to allow the user to filter by arbitrary column names, and we would agree: this would open up a relatively minor information leak if the user can sort on a field that’s not supposed to be exposed to them (for example, a field that contains the cost price of the item). But if the user truly is allowed to sort on any column from the table, this code should be safe: it would just cause an exception if the column doesn’t exist. In that respect, it behaves in a way similar to our first example.
By now you must have guessed that FuelPHP did’t escape identifiers correctly. Basically it simply wraps the identifier in (back)quotes, but that is trivial to break out of, of course. In fact, it’s worse: the sort direction string would be simply appended, as-is, to the SQL statement!