Re: Yet another fix for max_input_vars

From: Date: Fri, 06 Jan 2012 07:42:43 +0000
Subject: Re: Yet another fix for max_input_vars
References: 1 2 3 4  Groups: php.internals 
Request: Send a blank email to internals+get-57248@lists.php.net to get a copy of this message
Hi: so here is the problem, the number index collision can be fixed by this idea (increase table size with a random delta). now we need add the random number into DJB hash, and I am not good at math, so Calling for help, and the random number will be stored in a process global variable like: PHPAPI int zend_hash_random_number. and the reason for use a process global variable are: 1. this would break the zend hash cache 2. no abi backward break ( zend_hash_func) 3. simplify ZTS protection.. any help will be appreciated. thanks On Thu, Jan 5, 2012 at 10:21 PM, Laruence <laruence@php.net> wrote: > Hi, >  thanks to sesser, he point out that this won't work for string keys, > >  so, I guess, we should change the hash logic in the mean time.. I > will keep trying. > > thanks > > On Thu, Jan 5, 2012 at 9:12 PM, Etienne Kneuss <colder@php.net> wrote: >> 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 > > > > -- > Laruence  Xinchen Hui > http://www.laruence.com/ -- Laruence  Xinchen Hui http://www.laruence.com/

« previous php.internals (#57248) next »