An SQL injection against which prepared statements won't help

  1. A simple update helper
  2. The injection
  3. PDO::quote() - the wrong move
  4. String escaping - a disaster
  5. Adding backticks (still vulnerable)
  6. Escaping backticks
  7. Solution that's solid all around
  8. Comments (21)

A simple update helper

While toying with PDO, almost every PHP user ends up with sort of improvement for INSERT/UPDATE query, that accepts an associative array and creates a query with placeholders dynamically, which basically looks like this (a real code taken from a question on Stack Overflow):

$params = [];
$setStr = "";
foreach ($data as $key => $value)
{
    if ($key != "id")
    {
        $setStr .= $key." = :".$key.","; 
    }
    $params[':'.$key] = $value;

}
$setStr = rtrim($setStr, ",");
$pdo->prepare("UPDATE users SET $setStr WHERE id = :id")->execute($params);

It looks pretty good, as it can quickly produce a query out of any array which keys represent column names and values represent values to be used in the query. Given field names in HTML form are the same as table column names, it can greatly automate the form processing, letting you to use the same code to edit information in any table. Very convenient. But catastrophically vulnerable.

"How's that?" - you'd probably say - "placeholders are used and data is safely bound, therefore our query is safe". Yes, the data is safe. But in fact, what does this code do is taking user input and adding it directly to the query. Yes, it's a $key variable. Which goes right into your query untreated.

Which means you've got an injection.

The injection

Here is a little proof of concept code, which, as long as you have a table "users" and a row with id=1, will change the user name in the table to a result of SELECT query. And this query could be anything:

<form method=POST>
<input type=hidden name="name=(SELECT'hacked!')WHERE`id`=1#" value="">
<input type=hidden name="name" value="Joe">
<input type=hidden name="id" value="1">
<input type=submit>
</form>
<?php
if ($_POST) {
$pdo = new PDO('mysql:dbname=test;host=localhost', 'root', '');
    $params = [];
    $setStr = "";
    foreach ($_POST as $key => $value)
    {
        if ($key != "id")
        {
            $setStr .= $key." = :".$key.","; 
        }
        $params[$key] = $value; 

    }
    $setStr = rtrim($setStr, ",");
    $pdo->prepare("UPDATE users SET $setStr WHERE id = :id")->execute($params);
}    

This code will produce a query like this

UPDATE users SET name=(SELECT'hacked!')WHERE`id`=1# = :name=(SELECT'1')WHERE`id`=1#,name = :name WHERE id = :id

Where everything past first # will be treated as a comment.

You can try it at home.

What happens in this code? We are forging a form, adding another field to it, and writing an SQL code in the name attribute. The aforementioned helper function will take this forged SQL and insert it into constructed query. As a result, instead of "Joe" the name will be set to "hacked!". Not too scaring, eh? But you have to understand that what is dangerous here is the fact of injection. While as long as it is possible, the amount of exploits is infinite. And not all of them are that harmless. We will see a more dangerous exploit below.

PDO::quote() - the wrong move

Ok, we need to protect our useful code (as well as any other code that cannot be protected with prepared statements).

The first thing an average PHP user would probably think of is a built-in PDO::quote() function, which seemingly does what we need - protects the data from SQL injection. But soon it will be clear that a function intended for string formatting is inapplicable for identifiers. It will add quotes around returned value and will make our code to produce a sequence like this:

'name'='joe'

which will result in a syntax error - quotes are illegal as field name delimiters in MySQL.

DON'T ever think of trimming quotes from the resulting string either, as it was suggested some time ago in a heavily upvoted comment (now deleted) under PDO::quote()'s entry in PHP manual: the very purpose of this function is to do a complete formatting, doing both character escaping and adding quotes. Take out one of these two actions and you'll have an injection.

In other words, if we strip surrounding quotes from PDO::quote() result, it will be as though we only applied the regular sting escaping. And let's see why it is wrong:

String escaping - a disaster

Another thing a PHP user would think of is a familiar escaping function, which "makes your data safe" as falsely stated PHP manual for ages. Unfortunately, it will help only if injection code contains quotes or other characters this function deals with. The bad news, all these characters are unnecessary for injection, proving this function useless for the case (and disproving this function's fictional protecting skills):

<form method=POST>
<input type=hidden name="name=(SELECT password from admins)WHERE`id`=1#" value="">
<input type=hidden name="name" value="Joe">
<input type=hidden name="id" value="1">
<input type=submit>
</form>
<?php
if ($_POST) {
    $pdo = new PDO('mysql:dbname=test;host=localhost', 'root', '');
    $params = [];
    $setStr = "";
    foreach ($_POST as $key => $value)
    {
        if ($key != "id")
        {
            $setStr .= addslashes($key)." = :".$key.",";
        }
        $params[$key] = $value;

    }
    $setStr = rtrim($setStr, ",");
    $pdo->prepare("UPDATE users SET $setStr WHERE id = :id")->execute($params);
    var_dump("UPDATE users SET $setStr WHERE id = :id", $_POST);
}   

As you can see, there is not a single quote in the malicious query and thus neither addslashes() not various *_escape_string() can do anything here.

This time injection is more dangerous as it will reveal the admin's password to anyone.

Adding backticks (still vulnerable)

Ok, as we learned already, quotes won't help. May be adding backticks would help? Not a slightest:

<form method=POST>
<input type=hidden name="name`=(SELECT'hacked!')WHERE`id`=1#" value="">
<input type=hidden name="name" value="Joe">
<input type=hidden name="id" value="1">
<input type=submit>
</form>
<?php
if ($_POST) {
    $pdo = new PDO('mysql:dbname=test;host=localhost', 'root', '');
    $params = [];
    $setStr = "";
    foreach ($_POST as $key => $value)
    {
        if ($key != "id")
        {
            $setStr .= "`$key` = :$key,";
        }
        $params[$key] = $value;

    }
    $setStr = rtrim($setStr, ",");
    $pdo->prepare("UPDATE users SET $setStr WHERE id = :id")->execute($params);
} 

As you can see, adding backticks didn't help us at all: an attacker just adds another one to prematurely close the identifier and then proceed with malicious query. It happens because adding whatever delimiters around some literal is useless if we don't escape these delimiters inside - the lesson that we learned hard from conventional string-based injections. So let's escape our backticks!

Escaping backticks

So, what can be done to retain such a helpful function, but without a terrible breach?

As it was said before, escaping delimiters would help. Therefore, your first level of defense should be escaping delimiters (backticks) by doubling them. Assembling your query this way, 

$setStr .= "`".str_replace("`", "``", $key)."` = :".$key.",";

you will get the injection eliminated.

If we try to inject, using the method above, it will result in a query execution error, which, although is surely better than a successful injection, is still not quite a desirable behavior.

This is why quoting/escaping field names is still not enough. And this is why you should be using a another level of defense:

Solution that's solid all around

There is one more thing to keep in mind: although quoting/escaping field names will eliminate the classical SQL injection, there is another possible attack vector. As we don't control which field names are used for update, it is possible for the user to amend the value they shouldn't have an access to. For example, imagine there is a field in a users table like admin which is set to 1 for admins and 0 for regular users. The function in question will let any script-kiddie to raise the privilege level for their account.

Assuming all the above, such a useful function should be always accepting an additional parameter with a list of allowed field names: 

<form method=POST>
    <input type=hidden name="name`=(SELECT'hacked!')WHERE`id`=1#" value="">
    <input type=hidden name="name" value="Joe">
    <input type=hidden name="id" value="1">
    <input type=submit>
    </form>
<?php
if ($_POST) {
    $pdo = new PDO('mysql:dbname=test;host=localhost', 'root', '');
    $allowed = ["name","surname","email"];

    $params = [];
    $setStr = "";
    foreach ($allowed as $key)
    {
        if (isset($_POST[$key]) && $key != "id")
        {
            $setStr .= "`".str_replace("`", "``", $key)."` = :".$key.",";
            $params[$key] = $_POST[$key];
        }
    }
    $setStr = rtrim($setStr, ",");
    $params['id'] = $_POST['id'];
    var_dump("UPDATE users SET $setStr WHERE id = :id",$params, $_POST);
    $pdo->prepare("UPDATE users SET $setStr WHERE id = :id")->execute($params);
}

With such an improvement our code will become solid all around!

Of course, examples above would work only if emulation mode is turned on. But you have to understand that whether SQL injection can be successfully exploited or not is a different question and a quite irrelevant one. You should not let an injection in the first place. Or the way to exploit it will be found, this day or another.


Related articles:



Got a question?

I am the only person to hold a gold badge in , and on Stack Overflow and I am eager to show the right way for PHP developers.

Besides, your questions let me make my articles even better, so you are more than welcome to ask any question you have.

Click here to ask!

Latest article:
PHP Error Reporting

SEE ALSO:

Latest comments: