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