Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:57239
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.170 as permitted sender)
MIME-Version: 1.0
Sender: ekneuss@gmail.com
In-Reply-To: <CAF+90c-RNgGOwc8g0-DMfzN4HtzwLs3TWJAdyztFq6sZyasszQ@mail.gmail.com>
References: <CABBJUpe+BqyoUXbo=jN_V8N3RFrWt9gCA1s0LCztP0RFKD2+JA@mail.gmail.com>
	<CAF+90c-RNgGOwc8g0-DMfzN4HtzwLs3TWJAdyztFq6sZyasszQ@mail.gmail.com>
Date: Thu, 5 Jan 2012 14:12:58 +0100
Message-ID: <CACQesk6Oo23+827oOy8ExKEvt7ShdA8NVtdvrWmSX52vR-saSQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@googlemail.com>
Cc: Laruence <laruence@php.net>, PHP Internals <internals@lists.php.net>, 
	Rasmus Lerdorf <rasmus@lerdorf.com>, Stas Malyshev <smalyshev@sugarcrm.com>, 
	Pierre Joye <pierre.php@gmail.com>, =?UTF-8?Q?Johannes_Schl=C3=BCter?= <johannes@schlueters.de>, 
	Dmitry Stogov <dmitry@php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Yet another fix for max_input_vars
From: colder@php.net (Etienne Kneuss)

Hi,

On Thu, Jan 5, 2012 at 13:05, Nikita Popov <nikita.ppv@googlemail.com> wrot=
e:
> On Thu, Jan 5, 2012 at 9:53 AM, Laruence <laruence@php.net> wrote:
>> Hi:
>> =C2=A0 =C2=A0the origin thread is too long, so I open a new thread for t=
his.
>>
>> =C2=A0 =C2=A0I have made another patch try to fix this issue.
>>
>> =C2=A0 =C2=A0the key point is, randomizing the table size(tableMask).
>>
>> =C2=A0 =C2=A0instead of double the the table size in zend_hash_do_resize=
, =C2=A0I
>> increase the table size with a random delta ( the value now is just a
>> try, =C2=A0we can change it to a more appropriate value),
>> =C2=A0that is: =C2=A0new_table_size =3D old_table_size * 2 + random_num.
>>
>> =C2=A0 =C2=A0then, =C2=A0the collision can not be predicatible, which fi=
x the
>> problem, =C2=A0and also fixed the issue in json/soap/serialize etc.
>>
>> =C2=A0 =C2=A0here is the patch(draft now):
>> https://bugs.php.net/patch-display.php?bug_id=3D60655&patch=3Drand_hash_=
resize.patch&revision=3Dlatest
>>
>> =C2=A0 =C2=A0after this fix , may slow down the php, but compared to the
>> security threat I thinks the cost could be ignored.
>>
>> =C2=A0 =C2=A0for the test script list in
>> http://nikic.github.com/2011/12/28/Supercolliding-a-PHP-array.html:
>>
>> =C2=A0 =C2=A0*before patched:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0Inserting 65536 evil elements took 162.659402=
84729 seconds
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0Inserting 65536 good elements took 0.07555794=
7158813 seconds
>>
>> =C2=A0 =C2=A0*after =C2=A0patched:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0Inserting 65536 evil elements took 0.07412886=
6195679 seconds
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0Inserting 65536 good elements took 0.09104490=
2801514 seconds
>>
>> =C2=A0 =C2=A0what do you think ?
>
> At least one problem with your patch is that it uses crypto safe
> random numbers. The problem with that is that the very frequent random
> number fetches could deplete the entropy pool and thus make
> /dev/urandom (and probably the Windows RNG too) block. So you would
> again have a DOS vulnerability (just create many small arrays with 16
> elements so they get resized a few times). Additionally this could
> also impose a security threat to other application that rely on the
> entropy source.

In essence there should only be the need for one random number per
request, so it should be fine in that regard.

>
> Nikita
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>



--=20
Etienne Kneuss
http://www.colder.ch