Must we always escape values?
One of the cardinal rules of writing web applications is to escape user-generated input with functions like PHP’s real_escape_string. This is a great rule, but one that can have a negative impact on your application’s performance if used unnecessarily. For instance, when querying data with an integer parameter that is passed internally (not user-generated):
$query = "SELECT SQL_NO_CACHE * FROM `user` WHERE `user_id` = '" .
$mysqli->real_escape_string ( self::$user_id ) . "'";
$res = $mysqli->query ( $query );
The above code takes an average of 0.000922918319702 seconds to execute.
Whereas:
$query = "SELECT SQL_NO_CACHE * FROM `user` WHERE `user_id` = " . self::$user_id;
$res = $mysqli->query ( $query );
takes an average of only 0.000418901443481 seconds to execute.
Although the improvement is small (~0.0005 seconds), when your site runs millions (or tens-of-millions) of queries per day, the benefits begin to add up.
August 18th, 2008 at 2:09 pm
Of course numeric values are not passed through mysql_real_escape_string(), they are being casted to the appropriate numeric type (int|float) instead.
August 18th, 2008 at 2:26 pm
IMHO its best to default to security and require to actively disable those default security mechanism in a way that is searchable. For example I escape all variables send to templates, but I give the option of using the raw source as well. But the key thing is that when I use this option I have to do it in a way that I can search the code that loads templates for all those places, so that I can always quickly find those places and audit them for impact if I make changes. Like what happens if you decide you want natural keys instead of autogenerated keys? What happens if you then relied through out your code that the key is safe, because it has to be an “integer” (but this has to be an integer code was just ripped out).
August 18th, 2008 at 2:27 pm
How many security holes crop up in applications where the original programmers made the same “decision”.
You may be able to say right now that you are 100% certain that value can not be corrupted but in the future as other developers get involved, as modules or plugins are developed someone is going to make a simple mistake that won’t show up right away, It won’t break the application, won’t even cause your tests to puke but it will expose that “internal”
value to possible manipulation from the outside world!
Any when your application gets hacked and its traced back to this one unescaped value your going to slap yourself
August 18th, 2008 at 2:54 pm
The third test that I ran for this post was as follows:
if ( is_numeric ( self::$user_id ) ) {
$query = “SELECT SQL_NO_CACHE * FROM `user` WHERE `user_id` = ” . self::$user_id;
$res = $mysqli->query ( $query );
} else {
return FALSE;
}
In addition to allaying security concerns, this is also faster than using real_escape_string, clocking in at 0.000737087631226 seconds to execute:)
August 18th, 2008 at 3:08 pm
When dealing with integers, I almost recommend casting it in PHP. Not just the performance reasons, but also security.
For example, in _some_ rare instances you could be allowing math operations to be performed to bypass query restrictions. Such as:
SELECT fields FROM table WHERE field=12-1
That could a result in access to records the user normally wouldn’t have access to. Obviously, mysql_real_escape_string() won’t prevent this.
However, doing this would prevent that.
“SELECT fields FROM table WHERE field=” . (int)$field_id;
November 25th, 2008 at 9:46 am
Interesting post. I’ve also seen developers use a combination of sprintf and mysql_real_escape_string. As a matter of fact, here is an excerpt from the code example list in the PHP manual:
$query = sprintf(”SELECT * FROM users WHERE user=’%s’ AND password=’%s’”,
mysql_real_escape_string($user),
mysql_real_escape_string($password));
Thoughts?
–Keith