php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #60655 add max_input_vars for json/serialize
Submitted: 2012-01-04 15:26 UTC Modified: 2012-01-05 15:07 UTC
Votes:4
Avg. Score:4.0 ± 1.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: laruence@php.net Assigned:
Status: Open Package: *General Issues
PHP Version: 5.3.9RC4 OS:
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2012-01-04 15:26 UTC] laruence@php.net
Description:
------------
the max_input_vars restriction should also affect the json_encode and unserialize 


Patches

rand_hash_resize.patch (last revision 2012-01-05 08:09 UTC by laruence@php.net)
max_input_vars.patch (last revision 2012-01-05 05:04 UTC by laruence@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-01-04 15:26 UTC] laruence@php.net
-Type: Bug +Type: Feature/Change Request
 [2012-01-05 04:08 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: max_input_vars.patch
Revision:   1325736502
URL:        https://bugs.php.net/patch-display.php?bug=60655&patch=max_input_vars.patch&revision=1325736502
 [2012-01-05 04:17 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: max_input_vars.patch
Revision:   1325737023
URL:        https://bugs.php.net/patch-display.php?bug=60655&patch=max_input_vars.patch&revision=1325737023
 [2012-01-05 05:02 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: max_input_vars.patch
Revision:   1325739736
URL:        https://bugs.php.net/patch-display.php?bug=60655&patch=max_input_vars.patch&revision=1325739736
 [2012-01-05 05:03 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: max_input_vars.patch
Revision:   1325739809
URL:        https://bugs.php.net/patch-display.php?bug=60655&patch=max_input_vars.patch&revision=1325739809
 [2012-01-05 05:04 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: max_input_vars.patch
Revision:   1325739893
URL:        https://bugs.php.net/patch-display.php?bug=60655&patch=max_input_vars.patch&revision=1325739893
 [2012-01-05 08:09 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: rand_hash_resize.patch
Revision:   1325750958
URL:        https://bugs.php.net/patch-display.php?bug=60655&patch=rand_hash_resize.patch&revision=1325750958
 [2012-01-05 11:24 UTC] sesser@php.net
Your patch does not fix the problem.

It will make the first X hashtable grow operations random.
But the moment you already inserte 65536 entries the HashTable is now big enough 
to launch the attack.

Maybe your test script already breaks your patch the moment you try to insert 
2^17 entries.

Otherwise the attack script might need some tweaking. Anyway, your patch will 
not solve the problem.
 [2012-01-05 11:29 UTC] laruence@php.net
sorry, didn't get your point?  
the collision can not be predicatible any more, why this patch doesn't solve the 
problem?
 [2012-01-05 11:53 UTC] sesser@php.net
You are mistaken to believe that randomizing the TableSize will stop predictable 
collisions: This is only true if you try to exploit the problem with numerical 
indicies.

The moment you use alpha numerical keys and produce collisions in the DJB 
hashing function the table size does not matter anymore, because the return 
value of the hash function is the same.
 [2012-01-05 14:04 UTC] laruence@php.net
yes, the hash value of string index is the same, but the index = hash_value % 
nTableSize, 

we don't use the hash value as index directly, 

didn't I misunderstand you?
 [2012-01-05 14:05 UTC] laruence@php.net
oh, I got you, thanks.
 [2012-01-05 14:14 UTC] laruence@php.net
<laruence> I got you point, and agree in theory, yes, the string hash value could 
be the same, does anyone have a method to compute it in real?
<nikic> yes
<laruence> I really doubt that if we can find  so many string keys with the same 
hash value to be able launch a attach, and won't reach the max post size
 [2012-01-05 14:44 UTC] sesser@php.net
It is not "a theory", The whole disclosure from n-runs was about colliding the DJB 
hash function with alpha numerical keys.
 [2012-01-05 14:47 UTC] laruence@php.net
sesser, I am not good at algorithm, so if you can help me, I will appreciate.

just a guess, what about change the zend_hash_func, add some new seed like:

register ulong hash = 5381 + nKeyLength;

thanks
 [2012-01-05 14:48 UTC] sesser@php.net
BTW a simple approach to cause 65536 alpha numerical collisions would use most 
probably less than 2MB of POST payload. And this is the NOT mathematically 
optimized version.
 [2012-01-05 14:52 UTC] sesser@php.net
laruence: nothing against you, but fixing the hash table thing is not a simple 
easy fix. It must be done by someone who understands the mathematical problem of 
hash collisions and who understands the impact of making changes to a hash 
function.

Just changing some constants in the algorithm will not improve the situation. 
The opposite can be the case. By changing some constants it could be possible to 
destroy the distribution of collisions and suddenly some values collide more 
often than others.

So please do not try to fix a problem that must be solved by someone with the 
mathematical background knowledge.
 [2012-01-05 15:07 UTC] laruence@php.net
sesser, yes, you are right, a constant can not fix this problem, only make it more 
diffcult to find the special keys..
 [2012-01-05 18:13 UTC] miha dot vrhovnik at domenca dot si
As the authors say in the video the Perl solved this in 2003. They fixed it by randomizing hash seed. Probably for each "hash".
You can look this video for more details:
http://www.youtube.com/watch?v=R2Cq3CLI6H8&feature=channel_video_title
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Mon Apr 22 18:01:27 2019 UTC