Possible SQLi with self-made prepared statements and real_escape_string?

Question

My current assignment is to find a possible SQL injection in a PHP WebApp. While looking at the source code, I noticed that the way the script is handling prepared statments is weird.

$query = db::prepare("SELECT password FROM vault where id=%s", $_POST['id']);
$res = db::commit($query);

The interesting part from the prepare function looks like this:

    // escape
    foreach ($args as &$value){
        $value = static::$db->real_escape_string($value);
    }

    // prepare
    $query = str_replace("%s", "'%s'", $query);
    $query = vsprintf($query, $args);
    return $query;

Now, looking around for ways to bypass this, I noticed that single quotes are escaped, obviously because of the real_escape_string function. Looking around here, I found the following post which states that setting %s in single quotes preemptively looks highly suspect. However, I still haven't found any way how to exploit this, or if it is even exploitable.

Can anyone tell me, if there is anything wrong to escape and process user input like that? I know using the original mysqli/PDO prepared statement functions are a better idea, but given this isn't my code, it would be great to find out what exactly is wrong here and why you shouldn't do it like this.

Answer 1

if there is anything wrong to escape and process user input like that?

To clarify, there is nothing wrong with escaping per se as long as it is used on purpose - to escape not the arbitrary "user input" but specifically data to be used in the string literals in SQL query. Frankly, as long as the data in SQL query is both quoted and escaped, there should be no harm possible.

The code in question is a special case. Although bulk escaping is used (which is a big red flag as a rule), in this case it should be considered safe as long as only basic printf placeholders are used (i.e. %s, %d, %f etc), without argument swapping and other fancy stuff. As a result every input variable would be either

• wrapped in quotes if %s placeholder is used (and thus gets quoted and escaped to satisfy the rule from the above);
• or gets formatted as a number in case any other placeholder is used.

So I don't see any possibility for the injection.

The only slight security concern here is to use mysqli::set_charset() to set the connection encoding, to avoid a virtual vulnerability when a peculiar outdated encoding is used.

Answer 2

It could lead to problems which are not related directly to security. Because $value must be converted to a string before real_escape_string() can do its magic, all kind of localization problems could occur. Think about , instead of decimal point or date formats which won't be accepted by the database. It is not a good idea to handle all parameters as they where strings.