On 01/04/2012 08:13 PM, Laruence wrote:
> On Thu, Jan 5, 2012 at 5:56 AM, Rasmus Lerdorf <ras...@lerdorf.com> wrote:
>> On 01/04/2012 01:48 PM, Rasmus Lerdorf wrote:
>>> On 01/04/2012 01:27 PM, Stas Malyshev wrote:
>>>> Hi!
>>>>
>>>>> Right, like I said in my previous message, if this is caught by
>>>>> display_start_errors, I am ok with it. We need the default/no php.ini
>>>>> file case to not leak information like this.
>>>>
>>>> Just checked - it does not display error if display_startup_errors if
>>>> off, does display if it's on.
>>>
>>> Right, that seems ok. The other thing is that we need to clarify that it
>>> actually only limits the number of variables per nesting level. The
>>> current name and the description doesn't make that clear. You can still
>>> send 1M post vars in a single POST if you just nest them in a 1000x1000
>>> 2d array. Of course, this is likely going to hit the post_max_size
>>> limit, although many sites that do file uploads will have cranked that
>>> way up.
>>
>> Oh, and a final issue to address.
>>
>> This code:
>>
>> for($data=[],$i=0; $i<=999; $i++) $data[$i] = range(0,1001);
>> echo curl_post("http://localhost/index.php";,['a'=>$data]);
>>
>> will spew the warning 2000 times.
>>
>> & php post.php | grep Warning | wc -l
>> 2000
>>
> could you try with this new patch:
> https://bugs.php.net/patch-display.php?bug_id=60655&patch=max_input_vars.patch&revision=latest
> ?
>
> this patch also restrict the json / serializer , all of them must
> less than PG(max_input_vars).
>
> and different with the fix which was commited now, this patch count
> the num vars in a global scope, that means if there are 2 elements
> which both have 500 elements in post, the restriction will also
> affect,
>
> and this patch will not affect the existsing called to json or
> serializer, only affect the zif_json_decode and zif_serialize,
> after patch, the serialize will have a sencode parameter :"$max_vars".
>
> and the restriction can also be closed by set max_input_vars to 0.
Right, I don't think this is going to work. A simple 'make install'
after applying your patch fails with:
Warning: unserialize(): Unserialized variables exceeded 1000 in
phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
on line 1145
Warning: unserialize(): Unserialized variables exceeded 1000 in
phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
on line 1145
Warning: unserialize(): Unserialized variables exceeded 1000 in
phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
on line 1145
[PEAR] PEAR: pear.php.net/PEAR not installed
I really don't think this manual per-feature limiting is going to cut it
here. The real problem is the predictability of the hashing which we can
address by seeding it with a random value. That part is easy enough, the
hard part is figuring out where people may be storing these hashes
externally and providing some way of associating the seed with these
external caches so they will still work.
-Rasmus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
