Sanitizing input for SQL queries in Adodb for PHP

1.9k Views Asked by At

I'm optimizing a platform that uses ADODBforPHP. I used a sanitization function that avoids sql injections for previous versions of PHP (mysql_escape_string) which are obviously not longer supported nor recommended.

For those that haven't used the library, it goes something like this:

$rs = $cnn->Execute('SELECT * FROM user WHERE id_user='.q($_GET['id']));

Example when updating some row:

$record = array();
$record['name'] = q($_GET['name']);
$record['last_update'] = time();
$rsProfile = $cnn->Execute('SELECT * FROM user WHERE id_user='.q($_GET['id']));
$sql = $cnn->GetUpdateSQL($rsProfile,$record);
if($sql) $cnn->Execute($sql);                            

In this case, q($string) is the sanitize function, which i'm trying to improve. I don't have access to install PDO in this server, so that's not an option.

The current q() uses mysql_real_escape_string without the 2nd argument:

function q($data) {
    if(!empty($data) && is_string($data)) {
        $data = str_replace(array('\\', "\0", "\n", "\r", "'", '"', "\x1a"), array('\\\\', '\\0', '\\n', '\\r', "\\'", '\\"', '\\Z'), $data);
        $data = "'".$data."'";
    }
    return $data;
}

Someone recommended filter_var($value, FILTER_SANITIZE_STRING) on another forum, but I honestly haven't used that for these matters.

Any recommendations on how to improve the security of this function's purpose?

Update 1

function q($data) {
    if(is_string($data)) {
        return "'".mysql_real_escape_string($data)."'";
    } elseif(is_numeric($data) || is_bool($data)) {
        return $data;
    } else {
        return "''";
    }
}
2

There are 2 best solutions below

5
On BEST ANSWER

I am sorry for disappointing you, but your sanitization function, whatever it does, does not "sanitize" anything and you have an injection possible in the very code you posted here.
just call your script this way

code.php?id=1 union select password from users where id=1

and see if this code "sanitized" anything.

Any recommendations on how to improve the security of this function's purpose?

Sure.
First of all you have to understand what escaping is and how to use it.

Then you have to start using placeholders, I believe

3
On

From the documentation of mysql_escape_string:

This function became deprecated, do not use this function. Instead, use mysql_real_escape_string().

So, if you are using mysql, you should be just fine with mysql_real_escape_string.