Sanitizing input for SQL queries in Adodb for PHP

Question

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 "''";
    }
}

what's the point in using str_replace? why not to use mysql_escape_string? — Your Common Sense, Nov 01 '11 at 06:00
mysql_escape_string is already outdated and no longer supported. also, you can exploit that function by using %27. the solution is mysql_real_escape_string but that takes a 2nd argument (optional, but if it is null, it tries to generate a mysql connection- so, according to my point of view, it uses a few extra resources) — Andres SK, Nov 01 '11 at 13:47
the thing is, this system doesn't use the standard mysql_ functions. It uses ADODB for PHP, so there is no link identifier object, ever. — Andres SK, Nov 01 '11 at 14:51

Your Common Sense · Accepted Answer · 2011-11-01T06:05:08.827

2

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

edited Nov 01 '11 at 06:05

answered Nov 01 '11 at 05:54

Your Common Sense

156,878
40
214
345

1

thanks for your feedback. with the new update, if someone tries to inject something like that, it will return: SELECT * FROM tabla WHERE user='1 union select password from users where id=1' – Andres SK Nov 01 '11 at 14:08
I bet it will spoil your current queries where you have `''` quotes already – Your Common Sense Nov 01 '11 at 14:34
none of the current queries have ''. They all have ...field='.q($val) – Andres SK Nov 01 '11 at 14:50
1

maybe you didn't see the q() function. It adds the quotes automatically, depending on the datatype. If it is_numeric(), then no quotes are added. If it is a string, it escapes the characters and enclouses the whole value in quotes. – Andres SK Nov 01 '11 at 15:41
if some one would use ``p(id=1 union ' ' select password from users where id=1)`` the result would be, ``SELECT * FROM user WHERE id_user='id=1 union \' \' select password from users where id=1'``. Here the escaping and quoting works for sanitizing the way it should. And pls. don't mind the position of the additinal quotes, used here for demonstration purpose only. – LikeYou Nov 04 '14 at 00:44

score 0 · Answer 2 · answered Nov 01 '11 at 04:07

0

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.

answered Nov 01 '11 at 04:07

Hugo Mota

11,200
9
42
60

i am using MySQL but with the Adodb for PHP library. Because of that, i don't have the link_identifier for the 2nd parameter. The str_replace() you see in there, is the same mysql_real_escape_string but without the 2nd parameter. My question is: Is that enough for being protected against sql injections? – Andres SK Nov 01 '11 at 04:11
you can find discussions about this around here in stackoverflow. examples are [here](http://stackoverflow.com/questions/5304424/mysql-real-escape-string-not-good-enough) and [here](http://stackoverflow.com/questions/3123202/mysql-real-escape-string-is-it-enough-for-database-security-alone). and by the way: the second parameter of `mysql_real_escape_string` is optional. – Hugo Mota Nov 01 '11 at 04:18
as i read in the php documentation, it is optional, but if no such link is found, it will try to create one as if mysql_connect() was called with no arguments. If no connection is found or established, an E_WARNING level error is generated. Wouldn't that waste resources? – Andres SK Nov 01 '11 at 04:31

Sanitizing input for SQL queries in Adodb for PHP

2 Answers2