Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:57238
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain googlemail.com designates 209.85.215.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CABBJUpe+BqyoUXbo=jN_V8N3RFrWt9gCA1s0LCztP0RFKD2+JA@mail.gmail.com>
References: <CABBJUpe+BqyoUXbo=jN_V8N3RFrWt9gCA1s0LCztP0RFKD2+JA@mail.gmail.com>
Date: Thu, 5 Jan 2012 13:05:22 +0100
Message-ID: <CAF+90c-RNgGOwc8g0-DMfzN4HtzwLs3TWJAdyztFq6sZyasszQ@mail.gmail.com>
To: Laruence <laruence@php.net>
Cc: PHP Internals <internals@lists.php.net>, Rasmus Lerdorf <rasmus@lerdorf.com>, 
	Stas Malyshev <smalyshev@sugarcrm.com>, Pierre Joye <pierre.php@gmail.com>, 
	=?ISO-8859-1?Q?Johannes_Schl=FCter?= <johannes@schlueters.de>, 
	Dmitry Stogov <dmitry@php.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Yet another fix for max_input_vars
From: nikita.ppv@googlemail.com (Nikita Popov)

On Thu, Jan 5, 2012 at 9:53 AM, Laruence <laruence@php.net> wrote:
> Hi:
> =A0 =A0the origin thread is too long, so I open a new thread for this.
>
> =A0 =A0I have made another patch try to fix this issue.
>
> =A0 =A0the key point is, randomizing the table size(tableMask).
>
> =A0 =A0instead of double the the table size in zend_hash_do_resize, =A0I
> increase the table size with a random delta ( the value now is just a
> try, =A0we can change it to a more appropriate value),
> =A0that is: =A0new_table_size =3D old_table_size * 2 + random_num.
>
> =A0 =A0then, =A0the collision can not be predicatible, which fix the
> problem, =A0and also fixed the issue in json/soap/serialize etc.
>
> =A0 =A0here is the patch(draft now):
> https://bugs.php.net/patch-display.php?bug_id=3D60655&patch=3Drand_hash_r=
esize.patch&revision=3Dlatest
>
> =A0 =A0after this fix , may slow down the php, but compared to the
> security threat I thinks the cost could be ignored.
>
> =A0 =A0for the test script list in
> http://nikic.github.com/2011/12/28/Supercolliding-a-PHP-array.html:
>
> =A0 =A0*before patched:
> =A0 =A0 =A0 =A0Inserting 65536 evil elements took 162.65940284729 seconds
> =A0 =A0 =A0 =A0Inserting 65536 good elements took 0.075557947158813 secon=
ds
>
> =A0 =A0*after =A0patched:
> =A0 =A0 =A0 =A0Inserting 65536 evil elements took 0.074128866195679 secon=
ds
> =A0 =A0 =A0 =A0Inserting 65536 good elements took 0.091044902801514 secon=
ds
>
> =A0 =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.

Nikita