Apostrophes in cookies?

[Tristan Perry]

Hello all,
I've been testing the security of one of my old scripts. I've put $_COOKIE['x'] into SQL queries many times, without really realising there is anything bad about this.

Now, thinking about it, the script would be very insecure if apostrophes in cookies are allowed. When testing things, I can store a cookie like:

pass = ' or 1=1--'

Easily, although when I bring that variable into PHP, and use $_COOKIE['pass'] any apostrophes are escaped. Is this a default setting with PHP or am I just doing something wrong when handling the cookie? I ask because this would be a fairly big security hole and so I'd like to fix this if there is actually a problem. (I.e. If PHP doesn't normall escape apostraphes in cookies)
Thanks,
Tau Online

[[Unknown]]

It's just magic_quotes_gpc to the rescue again.

-[Unknown]

[Tristan Perry]

It's just magic_quotes_gpc to the rescue again.

-[Unknown]

Ah right, thanks. So to make things a bit more secure, doing something like:

Code Select Expand


if( !get_magic_quotes_gpc() )
  $_COOKIE['x'] = addslashes($_COOKIE['x']);

Before accessing the cookie would be a good idea?

[[Unknown]]

Indeed.  I prefer something like what SMF does, though.

-[Unknown]

[Chris Cromer]

Indeed.  I prefer something like what SMF does, though.

-[Unknown]

You mean filtering all cookies that are set with recursion? Personally I don't like that since not all cookies will be inserted into SQL or displayed in the html.

[[Unknown]]

But that way:
1. You can assume (policy) that magic_quotes_gpc is on, without ugly checks for it in your code.
2. It would be that way anyway if magic_quotes_gpc was on, so the alternative (with the above) is to recursively deslash all of them.
3. If you do it in-place, like Tau Online suggested, you could end up with doubly-slashed variables when magic_quotes_gpc is off (because the code gets run twice.)
4. If you DON'T do it in place, especially for long POST variables, you could end up eating a decent amount more of memory.
5. I'd rather have broken cookies than security holes: everyone will notice the broken cookies, but the security hole will mean flak and a new release.

-[Unknown]

[Chris Cromer]

You have 2 4's on your list.

1) I agree, but that's why I use a function to do the checks. So it's just one line of code:

Code Select Expand

$_COOKIE['x'] = escape_string($_COOKIE['x']);But with recursion it doesn't even need one line, since all of the cookies are sanatized already. But 1 line of code like that isn't necesarily ugly. But if it was a full check instead of a call to a function in that spot it would be ugly.

2) True although the check in the function handles that and doesn't add slashes if they where already added.

3) Um am a missing something? How would it be double slashed if Tau Online checked if magic_quotes_gpc was on or off before adding slashes? Well unless magic_quotes_runtime is on, but that wasn't mentioned.

4) I agree about that not doing it in place could eat up memory for long POST variables. That I think is the only downfall for recursion, is when it has to go through alot of data that has been submitted. The more user submitted data, the longer the recursion function runs.

5) That is a good point though, the mentality of better safe than sorry is something I 100% agree with.

[[Unknown]]

2. You missed my point.  That means you need two functions: escape and unescape.  Because, you can't do this:

echo $_POST['x'];

Because you *DON'T KNOW* (without deslashing) that it doesn't have slashes!  You have to deslash it there anyway, sometimes, which means it's an fcall either way.

3. Imagine:

$x = '\'';

for ($i = 0; $i < 2; $i++)
{
   if (get_magic_quotes_gpc() == 0)
      $x = addslashes($x);
}

echo $x;

How many slashes does it have now?  Sure, it looks dumb, but it really does happen.

4. Not really.  In most cases, people won't submit large amounts of array'd data to recurse over anyway.  If I was worried about recursion, I would have used a flat stack.

-[Unknown]

[Chris Cromer]

2) Yeah, can't really tell whether slashes have or have not been added already before.

3) Same could be said for the recursion system though. Imagine:

Code Select Expand

if (get_magic_quotes_gpc() == 0)
$x = addslashes($x);2 slashes added now since the recursion already added it once in another file. But they don't know about the recursion system and can't figure out why it's adding 2 slashes when magic_quotes_gpc is said to be off.

[[Unknown]]

That's why you have a policy of never adding slashes to POST/GET/etc.... and *never* checking get_magic_quotes_gpc.  That way, at least, if it adds too many slashes, it will do it everywhere, not on some servers.

-[Unknown]

[Tristan Perry]

Thanks for the help everyone. This topic has kind of made me realise that I know squat when it comes to PHP! Looking around a bit, it appears that recursion is when functions call themselves? That makes no sense to me, could someone shed some light on that? I found this article on recursion, although I didn't really find it too helpful. I'm right in thinking that QueryString.php is the file that SMF uses for recursion?

[Chris Cromer]

Yeah there is some functions in that file that use recursion.

Basically the function in SMF calls itself when it encounters an array, thus adding slashes to all the arrays within arrays within arrays, and so on for infinity(or till php crashes ). It keeps looping until it doesn't find any more embeded arrays to run through to add slashes.

Not all of the QueryString.php file uses recursion though, just certain functions in it.

I hope this helps... if not maybe [Unknown] could give a better example/explanation of what recursion is.

[Tristan Perry]

Yeah that helps a bit, thanks  How does a function call itself though? I've never really understood that, how would a function know that an array or whatever is being handled and call itself?

[Chris Cromer]

Well like this:

Code Select Expand

test();
function test() {
  test();
}

This is the most basic form of recursion possible. It just loops over and over till php crashes, or your browser times out. The first call is made from outside the function, but after that it calls itself each time it's run.

Alright here is an example of the array recursion, this one loops, but the loop doesn't go on forever:

Code Select Expand


$array = array('variable'=>
  array('variable2'=>
    array('variable3'=>"te'st")
  )
);

recursive_function($array);

function recursive_function($text) {
  // If not an array let's add slashes.
  if (!is_array($text)) {
    return addslashes($text);
  }

  // Alright we encountered an array, time for this function to call itself.
  foreach ($text as $index => $value) {
    // $value is the array, $index is the key.
    $text[$index] = recursive_function($value);
  }
}


This could should add slashes to the value in the inner most array. Making it te\'st instead of te'st.

[Tristan Perry]

Ah right, yeah that makes sense, thanks