On Thu, Jan 5, 2012 at 9:53 AM, Laruence <larue...@php.net> wrote:
> Hi:
>    the origin thread is too long, so I open a new thread for this.
>
>    I have made another patch try to fix this issue.
>
>    the key point is, randomizing the table size(tableMask).
>
>    instead of double the the table size in zend_hash_do_resize,  I
> increase the table size with a random delta ( the value now is just a
> try,  we can change it to a more appropriate value),
>  that is:  new_table_size = old_table_size * 2 + random_num.
>
>    then,  the collision can not be predicatible, which fix the
> problem,  and also fixed the issue in json/soap/serialize etc.
>
>    here is the patch(draft now):
> https://bugs.php.net/patch-display.php?bug_id=60655&patch=rand_hash_resize.patch&revision=latest
>
>    after this fix , may slow down the php, but compared to the
> security threat I thinks the cost could be ignored.
>
>    for the test script list in
> http://nikic.github.com/2011/12/28/Supercolliding-a-PHP-array.html:
>
>    *before patched:
>        Inserting 65536 evil elements took 162.65940284729 seconds
>        Inserting 65536 good elements took 0.075557947158813 seconds
>
>    *after  patched:
>        Inserting 65536 evil elements took 0.074128866195679 seconds
>        Inserting 65536 good elements took 0.091044902801514 seconds
>
>    what 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to