Everything

php

magic_quotes_gpc behaviour

php.internals

Arpad Ray

19 years ago

Hi all,

I'm curious about something with magic_quotes_gpc - it ignores the keys
of array values in $_GET etc, despite escaping keys of scalar values and
all keys in contained arrays. For example, the query string ?a'b=1
yields $_GET[a\'b] = 1, but ?a'b[a'b]=1 yields $_GET[a'b][a\'b] = 1.

While many other aspects of magic_quotes_gpc have changed, this
behaviour seems to have stayed the same since at least PHP 4.2.0, see:
http://www.rajeczy.com/compat_gpc_tests.txt

So, is this behaviour deliberate, and if so, what's the rationale?

Arpad

Arpad Ray

19 years ago

Arpad Ray wrote:

> So, is this behaviour deliberate, and if so, what's the rationale?
>

The problem seems to be in (5.2.x CVS) php_variables.c, lines 161-166:

if (PG(magic_quotes_gpc) && (index != var)) {
/* no need to addslashes() the index if it's the
main variable name */
escaped_index = php_addslashes(index, index_len,
&index_len, 0 TSRMLS_CC);
} else {
escaped_index = index;
}

If "&& (index != var)" is removed, all keys are escaped as expected. The
equivalent line for keys of scalar variables (198) lacks this check
since 5.0.0, and since then it has escaped them correctly. It's still
there in the 4.4 branch.

Is there any reason not to remove this check, at least in the 5.2 branch?

Arpad

David Zülke

19 years ago

Am 09.04.2007 um 15:41 schrieb Arpad Ray:

> Arpad Ray wrote:
>> So, is this behaviour deliberate, and if so, what's the rationale?
>>
>
> The problem seems to be in (5.2.x CVS) php_variables.c, lines 161-166:
>
>                if (PG(magic_quotes_gpc) && (index != var)) {
>                    /* no need to addslashes() the index if it's the
> main variable name */
>                    escaped_index = php_addslashes(index, index_len,
> &index_len, 0 TSRMLS_CC);
>                } else {
>                    escaped_index = index;
>                }
>
> If "&& (index != var)" is removed, all keys are escaped as
> expected. The equivalent line for keys of scalar variables (198)
> lacks this check since 5.0.0, and since then it has escaped them
> correctly. It's still there in the 4.4 branch.
>
> Is there any reason not to remove this check, at least in the 5.2
> branch?

Yes, BC. magic_quotes is crappy/complicated enough to deal with
already, please don't make it behave differently between PHP5
versions. No need to. Nobody should use it anymore, so there is no
reason to change behavior again anyway.


David

Arpad Ray

19 years ago

David Zülke wrote:

> Yes, BC. magic_quotes is crappy/complicated enough to deal with
> already, please don't make it behave differently between PHP5
> versions. No need to. Nobody should use it anymore, so there is no
> reason to change behavior again anyway.
>

What we seem to have here is a bug in a currently supported feature. You
say twice that there's no reason to fix it, but I'd say that it's a
rather crucial issue for anyone still using magic quotes. Of course
they're already more vulnerable than not if they're using magic quotes,
but this opens up yet another hole which we can easily fix.

The fact that nobody "should" be using it anymore is more of a matter
for PHP6, from which it's been removed already.

By the way, there have already been BC changes with magic quotes in PHP5
- namely keys being escaped when it was turned off between 5.0.0 and 5.1.0.

Arpad

Arpad Ray

19 years ago

Update: I raised this issue as a bug (#41093) because of lack of
interest here. It's now been fixed in CVS and will therefore be in 5.2.2
RC2 (thanks Ilia).

Arpad