Using mysql_real_escape_string and sprintf

Hello to everyone.

I am hoping someone will be able to clear up something I have been wondering about. I understand what mysql_real_escape_string does and why it is necessary to clean inputs before passing them to an sql query. What I don’t understand is why the php manual shows the sql query being cleaned by mysql_real_escape_string all wrapped inside sprintf. I can’t work out why this query

$sql = 'select * from users where username = "'.mysql_real_escape_string($username,$con).'" and password = "'.sha1(mysql_real_escape_string($password)).'"'

might be more prone to sql injection if it isn’t wrapped in sprintf?

Is this query just as safe from sql injection as it would be if it was wrapped in sprintf?

mysql_real_escape_string is not to do with inputs. It’s to protect mysql from the values being outputted to the database.

sprintf provides no injection protection. It just allows you to more easily read the SQL code than if the variables were embedded within it.

Compare these two:


$sql = 'SELECT * FROM users WHERE username = "'.mysql_real_escape_string($username,$con).'" ' .
    'AND password = "'.sha1(mysql_real_escape_string($password)).'"';


$sql = sprintf('SELECT * FROM users WHERE username = "%s" AND password = "%s"',
    mysql_real_escape_string($username,$con),
    sha1(mysql_real_escape_string($password))
);

Thanks for clearing it up :slight_smile: That’s what I was hoping was the case as I haven’t been using sprintf up till now.

Isn’t running real escape string to a password before hashing it kind of detrimental to the whole process? I mean, it won’t hurt anything since it’ll be in alphanumeric form, and in some cases it would make a users password no longer work? Or am I way off on this one? :confused:

That’s right. If it’s used at all, mysql_real_escape_string should be as an outside wrapper, not an inside one.

And it isn’t needed at all when you know that there is nothing in the data that can be confused with the SQL (eg, a hashed password can never contain any character that would allow SQL injection and so escaping it is unnecessary).

I think that we’re both agreeing about the same thing here. Not to pull a Kalon on you, but reread what I said, where after agreeing that it shouldn’t be used in this hashing situation, I follow on with “if it’s used at all” which is supposed to refer to a completely different situation not involving this hashing situation.

Yes. I was giving an alternate explanation so as to add emphasis to the point you were making since so many people use that function when it isn’t necessary.

Some people believe it is good practice to always escape strings for queries so that it becomes a habit. Personally, I always escape strings if the value is not created in the same or previous line in the code (=it is not visible right away where the value comes from). Of course in the given example it’s obvious, but when in doubt it’s good to escape by default.

Imagine if you could do something like this :

$db->setQuery('SELECT * FROM users WHERE username = ":user" AND password = ":pass"');
$db->setData(array('user'=> $username, 'pass' => $password));

And this “setData” thing would make sure that your data is ok for your database. It would be so much better.