Re: Re: another fix for max_input_vars.

On Wed, Jan 4, 2012 at 6:18 PM, Stas Malyshev <smalyshev@sugarcrm.com>wrote: > Hi! > > > Hi: >> I have updated the patch, make it works in case of sub arrays. >> >> http://pastebin.com/yPTUZuNe >> > > I'm sorry, I'm not sure still I understand - what is this patch fixing? > I.e. what is the problem with the current PHP that needs this patch? > > the max_input_vars was introduced to fix http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-4885 My understanding is that the original patch was too intrusive, from the comment of Laruence, it seems that throwing an E_ERROR in that phase means that you can kill the cgi workers if you pass the malicious input, which has still some DOS potential, this seems to be backed as Dmitry changed the original patch, to only raise a warning and ignore the vars over the set limits: http://svn.php.net/viewvc?view=revision&revision=321335 From the comments by Laruence it seems that he thinks that we only need to limit post, as get and cookie has external limits. I disagree with this, for two reasons: - we also have post_max_size to limit the length of the Post Content-Length, so by the same logic, we wouldn't need an additional ini setting for limiting the number of variables. - we also have max_input_nesting_level, so having a max_input_vars seems to be more consistent, than max_post_vars I would also like to point out, that when we added max_input_nesting_level we later polished that a little bit: http://svn.php.net/viewvc/php/php-src/trunk/main/php_variables.c?r1=233940&r2=236894 http://svn.php.net/viewvc/php/php-src/trunk/main/php_variables.c?r1=236898&r2=239877 And I guess at least the information disclosure part would be needed here also ( https://twitter.com/#!/i0n1c/status/152356767601393665 ) -- Ferenc Kovács @Tyr43l - http://tyrael.hu