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

[Sebastian Mendel]

Marc Delisle schrieb:
> 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).

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