Sebastian Mendel a écrit :

Michal Čihař schrieb:

...

Hi

On Thu, 01 Mar 2007 15:30:59 +0100 Sebastian Mendel <lists@sebastianmendel.de> wrote:

...

http://www.php-security.org/MOPB/MOPB-02-2007.html

i did not fully 'understand' how we are affected, but i think we are affected somehow ... especially as i come to the sentence wehre phpMyAdmin is explicitely mentioned ... This is IMHO PHP problem and causes problems because single line of our code gets executed...

yes of course it is a PHP problem ... but the globals overwrite is also a PHP problem and we do check for this ...

a simple counter wuld help, or?

teh only place where we would be possible attackable with this is when we iterate over $GLOBALS or $_REQUEST ($_POST, $_COOKIE, $_GET)

common.lib.php#2651 /** * Check for numeric keys * (if register_globals is on, numeric key can be found in $GLOBALS) */ $i = 0; foreach ($GLOBALS as $key => $dummy) { if (++$i >= 1000) { die('possible deep recurse attack'); } if (is_numeric($key)) { die('numeric key detected'); } }

and

/** * calls $function vor every element in $array recursively * * @uses PMA_arrayWalkRecursive() * @uses is_array() * @uses is_string() * @param array $array array to walk * @param string $function function to call for every array element */ function PMA_arrayWalkRecursive(&$array, $function, $apply_to_keys_also = false) { static $recursive_counter = 0; if (++$recursive_counter > 1000) { die('possible deep recursion attack'); } foreach ($array as $key => $value) { if (is_array($value)) { PMA_arrayWalkRecursive($array[$key], $function, $apply_to_keys_also); } else { $array[$key] = $function($value); }

if ($apply_to_keys_also && is_string($key)) { $new_key = $function($key); if ($new_key != $key) { $array[$new_key] = $array[$key]; unset($array[$key]); } } } $recursive_counter--; }

what would be a good value? 10.000? but we never will need such much vars, so even 1.000 would be enough? (count all all variables that be available when register_globals = on)

Yes, I was thinking about adding a limit, your analysis seems OK to me. A limit of 1000 is enough (even a smaller value would be correct like 100 I guess). Did you test this patch? Marc

Sebastian Mendel a écrit :

Marc Delisle schrieb:

...

Sebastian Mendel a écrit :

...

Michal Čihař schrieb:

...

Hi

On Thu, 01 Mar 2007 15:30:59 +0100 Sebastian Mendel <lists@sebastianmendel.de> wrote:

...

http://www.php-security.org/MOPB/MOPB-02-2007.html

i did not fully 'understand' how we are affected, but i think we are affected somehow ... especially as i come to the sentence wehre phpMyAdmin is explicitely mentioned ... This is IMHO PHP problem and causes problems because single line of our code gets executed... yes of course it is a PHP problem ... but the globals overwrite is also a PHP problem and we do check for this ...

a simple counter wuld help, or?

teh only place where we would be possible attackable with this is when we iterate over $GLOBALS or $_REQUEST ($_POST, $_COOKIE, $_GET)

common.lib.php#2651 /** * Check for numeric keys * (if register_globals is on, numeric key can be found in $GLOBALS) */ $i = 0; foreach ($GLOBALS as $key => $dummy) { if (++$i >= 1000) { die('possible deep recurse attack'); } if (is_numeric($key)) { die('numeric key detected'); } }

and

/** * calls $function vor every element in $array recursively * * @uses PMA_arrayWalkRecursive() * @uses is_array() * @uses is_string() * @param array $array array to walk * @param string $function function to call for every array element */ function PMA_arrayWalkRecursive(&$array, $function, $apply_to_keys_also = false) { static $recursive_counter = 0; if (++$recursive_counter > 1000) { die('possible deep recursion attack'); } foreach ($array as $key => $value) { if (is_array($value)) { PMA_arrayWalkRecursive($array[$key], $function, $apply_to_keys_also); } else { $array[$key] = $function($value); }

if ($apply_to_keys_also && is_string($key)) { $new_key = $function($key); if ($new_key != $key) { $array[$new_key] = $array[$key]; unset($array[$key]); } } } $recursive_counter--; }

what would be a good value? 10.000? but we never will need such much vars, so even 1.000 would be enough? (count all all variables that be available when register_globals = on) Yes, I was thinking about adding a limit, your analysis seems OK to me. A limit of 1000 is enough (even a smaller value would be correct like 100 I guess).

yes, i thought about 100 also first! but this is too low - i have found without any $_REQUEST 211 vars ...

function myCount($var) { static $count = 0; $count++; if (is_array($var)) { foreach ($var as $name => $each_var) { if ($name !== 'GLOBALS') { myCount($each_var); } } } $GLOBALS['count'] = $count; }

myCount($GLOBALS);

var_dump($count);

i think with 1000 we are on the safe side ...

...

Did you test this patch?

no - i have no linux where i can do easily this magic call with thousends of vars ... ;-)

curl http://127.0.0.1/phpmyadmin/ -d a`php -r 'echo str_repeat("[a]",1001);'`=1

Ok, if you want to commit to trunk, I'll test here. Then we'll probably release 2.10.0.2. Now, what do we do with http://sourceforge.net/tracker/index.php?func=detail&aid=1647030&group_id=23... they want security fixes published as patches. It's more work for us, but I can understand distro maintainers. Marc

Sebastian Mendel a écrit :

Marc Delisle schrieb:

...

Ok, if you want to commit to trunk, I'll test here. Then we'll probably release 2.10.0.2.

if have commited to trunk

Fix confirmed!

...

Now, what do we do with http://sourceforge.net/tracker/index.php?func=detail&aid=1647030&group_id=23...

they want security fixes published as patches. It's more work for us, but I can understand distro maintainers.

i have created this patch adn attached to the bug report

OK, so in our security advisory we can refer to this. I'll ensure to attach a patch that is applicable to 2.10.0.1. Marc

Marc Delisle schrieb:

Sebastian,

this part of the patch: /** + * protect against deep recursion attack CVE-2006-1549, + * 1000 seems to be more than enough + * + * @see http://www.php-security.org/MOPB/MOPB-02-2007.html + * @see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-1549 + */ +if (count($GLOBALS) > 1000) { + die('possible deep recurse attack'); +}

is not reached when I test the attack of MOPB-02, it's the other part that protects for this attack.

Do you know in which case this code would trigger? In the case of an attempt to override $GLOBALS?

it should trigger if and only if register_globals is on

Marc Delisle schrieb:

Sebastian Mendel a écrit :

...

Marc Delisle schrieb:

...

Sebastian,

this part of the patch: /** + * protect against deep recursion attack CVE-2006-1549, + * 1000 seems to be more than enough + * + * @see http://www.php-security.org/MOPB/MOPB-02-2007.html + * @see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-1549 + */ +if (count($GLOBALS) > 1000) { + die('possible deep recurse attack'); +}

is not reached when I test the attack of MOPB-02, it's the other part that protects for this attack.

Do you know in which case this code would trigger? In the case of an attempt to override $GLOBALS?

it should trigger if and only if register_globals is on

I cannot make this code trigger when register_globals is on, it's always the protection in PMA_arrayWalkRecursive() that triggers.

I'm attacking with curl http://127.0.0.1/phpmyadmin/ -d a`php -r 'echo str_repeat("[a]",20000);'`=1

do you have some other attack in mind?

this will trigger with phpmyadmin/?1=1;2=2;3=3;...;100000=100000 this would also be triggered inside PMA_arrayWalkRecursive() but at this point we could have allready iterated over $GLOBALS ... -- Sebastian Mendel www.sebastianmendel.de

Marc Delisle schrieb:

Sebastian Mendel a écrit :

...

Michal Čihař schrieb:

...

Hi

On Thu, 01 Mar 2007 15:30:59 +0100 Sebastian Mendel <lists@sebastianmendel.de> wrote:

...

http://www.php-security.org/MOPB/MOPB-02-2007.html

i did not fully 'understand' how we are affected, but i think we are affected somehow ... especially as i come to the sentence wehre phpMyAdmin is explicitely mentioned ... This is IMHO PHP problem and causes problems because single line of our code gets executed... yes of course it is a PHP problem ... but the globals overwrite is also a PHP problem and we do check for this ...

a simple counter wuld help, or?

teh only place where we would be possible attackable with this is when we iterate over $GLOBALS or $_REQUEST ($_POST, $_COOKIE, $_GET)

common.lib.php#2651 /** * Check for numeric keys * (if register_globals is on, numeric key can be found in $GLOBALS) */ $i = 0; foreach ($GLOBALS as $key => $dummy) { if (++$i >= 1000) { die('possible deep recurse attack'); } if (is_numeric($key)) { die('numeric key detected'); } }

and

/** * calls $function vor every element in $array recursively * * @uses PMA_arrayWalkRecursive() * @uses is_array() * @uses is_string() * @param array $array array to walk * @param string $function function to call for every array element */ function PMA_arrayWalkRecursive(&$array, $function, $apply_to_keys_also = false) { static $recursive_counter = 0; if (++$recursive_counter > 1000) { die('possible deep recursion attack'); } foreach ($array as $key => $value) { if (is_array($value)) { PMA_arrayWalkRecursive($array[$key], $function, $apply_to_keys_also); } else { $array[$key] = $function($value); }

if ($apply_to_keys_also && is_string($key)) { $new_key = $function($key); if ($new_key != $key) { $array[$new_key] = $array[$key]; unset($array[$key]); } } } $recursive_counter--; }

what would be a good value? 10.000? but we never will need such much vars, so even 1.000 would be enough? (count all all variables that be available when register_globals = on)

Yes, I was thinking about adding a limit, your analysis seems OK to me. A limit of 1000 is enough (even a smaller value would be correct like 100 I guess).

yes, i thought about 100 also first! but this is too low - i have found without any $_REQUEST 211 vars ... function myCount($var) { static $count = 0; $count++; if (is_array($var)) { foreach ($var as $name => $each_var) { if ($name !== 'GLOBALS') { myCount($each_var); } } } $GLOBALS['count'] = $count; } myCount($GLOBALS); var_dump($count); i think with 1000 we are on the safe side ...

Did you test this patch?

no - i have no linux where i can do easily this magic call with thousends of vars ... ;-) curl http://127.0.0.1/phpmyadmin/ -d a`php -r 'echo str_repeat("[a]",1001);'`=1 -- Sebastian Mendel www.sebastianmendel.de

Marc Delisle schrieb:

Sebastian Mendel a écrit :

...

http://www.php-security.org/MOPB/MOPB-02-2007.html

i did not fully 'understand' how we are affected, but i think we are affected somehow ... especially as i come to the sentence wehre phpMyAdmin is explicitely mentioned ...

We recursively call PMA_gpc_extract(), maybe we can do something about this as a workaround to this PHP problem.

To better see what happens, add a print_r() like this:

function PMA_gpc_extract($array, &$target, $sanitize = true) { print_r($array); if ( ! is_array($array) ) { return false; }

but this comes after the iteration over $GLOBALS and PMA_arrayWalkRecursive($_GET, 'stripslashes', true); PMA_arrayWalkRecursive($_POST, 'stripslashes', true); PMA_arrayWalkRecursive($_COOKIE, 'stripslashes', true); PMA_arrayWalkRecursive($_REQUEST, 'stripslashes', true); so this is too late ... ;-)

and call a modified version of the exploit

curl http://127.0.0.1/phpmyadmin/ -d a`php -r 'echo str_repeat("[a]",5);'`=1

-- Sebastian Mendel www.sebastianmendel.de