3

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.

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.

0

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.

  • Regarding a comma, wouldn't it be the same with any other method as well? A number will be cut down and a date would be sent as is if, for example, a native prepared statement is used – Your Common Sense Dec 20 '18 at 17:27
  • @YourCommonSense - It is my understanding, that when we use prepared statements, the arguments are sent separately to the database with their original datatype. The database itself doesn't have to insert them into the sql command, so the variables are never converted to a string. – martinstoeckli Dec 20 '18 at 17:32
  • Yes, but dates and numbers with a comma are strings already – Your Common Sense Dec 20 '18 at 18:07
  • @YourCommonSense - The function real_escape_string() expects a string, so it will convert a double parameter to a string with the localization settings of the PHP environment (possibly ,). The database expects a . in Sql though. There is no need to call real_escape_string() when we use prepared statements, so there is no conversion, we can add the double to the parameters directly. The OPs code uses parametrized queries, but not prepared statements. – martinstoeckli Dec 20 '18 at 21:30
  • Ah yes. You are right, I see your point. For the floating point numbers it's indeed could be the case if default locale settings are changed. – Your Common Sense Dec 21 '18 at 6:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.