Is this PHP code exploitable?

Question

Possible Duplicate:
Are mysql_real_escape_string() and mysql_escape_string() sufficient for app security?

So basically I have a qryName in the URL

eg: mysite.com/qryName=WHAT

if (isset($_GET['qryName']))
{
    $qryName = mysql_real_escape_string(filter($_GET['qryName']));
}

$urldata = mysql_fetch_assoc(mysql_query("SELECT * FROM gangs WHERE ShortName = '" . $qryName . "' LIMIT 1"));

$urldata is the code so it pretty much selects from the database. Note that in the database, the qryName has no spaces, special chars etc..

Just wondering if that is exploitable?

Learn PDO. `mysql_*` functions are officially deprecated due to security concerns. — itachi, May 11 '12 at 14:38
@itachi please show how `mysql_real_escape_string()` is unsafe in this scenario — Pekka, May 23 '12 at 21:14

score 1 · Answer 1 · answered May 11 '12 at 14:26

It is safe since you properly escape the value - unless....

...you do not initialize the variable and have register_globals enabled. In that case someone can use a cookie or POST value to send you an arbitrary value for $qryName containing evil SQL statements.

But since you probably just posted a snipped and do initialize the variable before that if statement (you do, right?!), your code is safe. Consider using prepared statements (with PDO) though instead of escaping - they make your code more readable, too.

Matt · Answer 2 · 2012-05-11T14:35:04.410

Have you considered using something like PDO? My understanding is that when using PDO and bound variables, SQL injection is not possible. There are also other advantages worth considering.

A similar PDO query would be:

    $data=array($_GET['qryName']);
    try {
        $STH = $this->DBH->prepare('SELECT * FROM gangs WHERE ShortName = ? LIMIT 1');
        $STH->execute($data);
        while($row = $STH->fetch()) {
                $var1=$row->FieldName;
        }
    }
    catch(PDOException $e) {echo $e->getMessage();}

You add the variables to the array ($data) and they are bound in order to each question mark in the SQL statement.

score 1 · Answer 3 · answered May 11 '12 at 14:31

Why don't you add one extra piece of validation or take out the isset and check if it only contains letters for example

if(ctype_alpha($_GET['qryName'])) {

    $qryName = mysql_real_escape_string(filter($_GET['qryName']));

}

http://php.net/manual/en/function.ctype-alpha.php

Is this PHP code exploitable?

3 Answers3