[Phpmyadmin-devel] MOPB-02-2007 deep recursion,

[Marc Delisle]

Sebastian Mendel a écrit :
> Michal Čihař schrieb:
>> Hi
>>
>> On Thu, 01 Mar 2007 15:30:59 +0100
>> Sebastian Mendel <lists at 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