Re: Yet another fix for max_input_vars

Hi, On Thu, Jan 5, 2012 at 13:05, Nikita Popov <nikita.ppv@googlemail.com> wrote: > On Thu, Jan 5, 2012 at 9:53 AM, Laruence <laruence@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. 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 > -- Etienne Kneuss http://www.colder.ch