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

[Marc Delisle]

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 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
> 
> 

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=23067&atid=377408

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

Marc