Escaping cookie values to prevent sql injection

Question

I read this comment on the mysql_real_escape_string php documentation page:

Also don't forget to escape $_COOKIE array before querying the database. In firefox you can edit cookies and insert and inject harmful sql queries.

<?php 
  foreach ($_COOKIE as $key => $value) { 
    if(get_magic_quotes_gpc()) $_COOKIE[$key]=stripslashes($value); 
    $_COOKIE[$key] = mysql_real_escape_string($value); 
  } 
?>

Am I right in thinking I only have to do this if I use these cookie values in a query? So if no sql statement uses values from these cookies there is no need to escape the cookies like above?

I am using mysql_query not prepared statements (all the inhouse company code I am working with uses mysql_query)

It'd be a pointless operation. strip_slashes and mysql_real_escape_string are **NOT** inverses of each other, and this code would potentially double/triple/.../N-th escape the data, every time it runs. You only escape/quote data that's going to be used IMMEDIATELY in a query, not "maybe sometime later", and then you do NOT overwrite the original with the quoted version. — Marc B, May 31 '13 at 14:14

score 2 · Answer 1 · edited May 23 '17 at 11:43

Am I right in thinking I only have to do this if I use these cookie values in a query?

Yes... ish.

You shouldn't overwrite superglobals with data that is only fit for stuffing in a MySQL query. Do escaping at the last minute and to local variables.

There are better ways to defend against SQL injection then variable escaping anyway.

I am using mysql_query not prepared statements (all the inhouse company code I am working with uses mysql_query)

I suggest starting the migration process. It will be less painful then having to do it all in one go when you want to upgrade to a future version of PHP that doesn't have the deprecated library in it.

score 2 · Accepted Answer · answered May 31 '13 at 14:09

You just need to remember this concept once: Whenever you concatenate one string into a special text format, you need to escape it according to that format at the moment of concatenation. It doesn't matter where that value comes from. You do not escape values before or after concatenating them, but right when you do. You do not blanket escape your cookie values just because. You escape a value you want to put into an SQL query right when you put it in there. Cookies are no special case; it's sad enough that the reminder needs to be there.

Your code is always:

$sql = sprintf("SELECT ... WHERE foo = '%s'", mysql_real_escape_string($var));

It's not:

$var = mysql_real_escape_string($var);

// 100 lines of irrelevant code

$sql = "SELECT ... WHERE foo = '$var'";

Read The Great Escapism (Or: What You Need To Know To Work With Text Within Text).

score 1 · Answer 3 · answered May 31 '13 at 14:09

Yes, you are right in assuming this is only required when using them in database queries.
No, you should definitely not do it globally for ALL cookies.
No, you should never concatenate cookie values in SQL queries anyway, use prepared statements.
mysql_real_escape_string is deprecated with its entire family of mysql_ functions since PHP 5.5. Switch to PDO or MySQLi instead. And use their faciliteit for prepared statements to remove all worries about SQL injection.
You can edit cookies in every browser, not just Firefox.

score 0 · Answer 4 · answered May 31 '13 at 14:08

Am I right in thinking I only have to do this if I use these cookie values in a query? So if no sql statement uses values from these cookies there is no need to escape the cookies like above?

yes, of course ;)

I am using mysql_query not prepared statements (all the inhouse company code I am working with uses mysql_query)

Try to migrate to prepared statements with either PDO or mysqli

If you cannot migrate: Note that you'll have to establish the mysql connection before you call mysql_real_escape_string()! That's because the function uses the current connection encoding to properly escape strings. If you connect afterwards the escaping might be wrong.

score 0 · Answer 5 · answered May 31 '13 at 14:10

This reminder is there to tell you, that you can't trust values you get from cookies, even if you set them yourself, because anyone can alter them. So, yes.

Of course you should not use mysql_ functions and all that, but I think you know that (have to use them in an older application, too).

Escaping cookie values to prevent sql injection

5 Answers5