Count collisions during HT insertion operations #1565
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| --TEST-- | ||
| Bug #70644: trivial hash complexity DoS attack (plain array) | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| $a = []; | ||
| $s = 1 << 16; | ||
| for ($i = 0; count($a) < $s; $i += $s) { | ||
| $a[$i] = 0; | ||
| } | ||
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Fatal error: Too many collisions in hashtable in %s on line %d |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --TEST-- | ||
| Bug #70644: trivial hash complexity DoS attack (json_decode) | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| $s = 1 << 16; | ||
| $a = []; | ||
| for ($i = 0, $j = 0; $i < $s; $i++, $j += $s) { | ||
| $a[] = '"' . $j . '": null'; | ||
| } | ||
| $j = '{' . implode(', ', $a) . '}'; | ||
| var_dump(json_decode($j, true)); | ||
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Fatal error: Too many collisions in hashtable in %s on line %d | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -459,12 +459,25 @@ ZEND_API void ZEND_FASTCALL _zend_hash_iterators_update(HashTable *ht, HashPosit | |
| } | ||
| } | ||
|
|
||
| /* To protect against HashDos attacks, we count collisions during the "find | ||
| * existing bucket with this key" phase of insertion operations. The check | ||
| * against MAX_COLLISIONS is only performed if *no* matching bucket is found, | ||
| * as that corresponds to insert operations (as opposed to updates). | ||
| * | ||
| * An important caveat of the implementation is that collisions will *not* | ||
| * be counted for add_new operations. find+add_new operations should be replaced | ||
| * with add_or_return operations. | ||
| */ | ||
| #define HT_MAX_COLLISIONS 1000 | ||
|
There was a problem hiding this comment. What happens if a developer runs into this issue with an application without a malicious intention? Should this maybe a ini setting? Or is this case really absolutely hypothetical (I don't have a in-depth understanding of the possibility of this happening outside actual DOS attempts) ? Thank you! There was a problem hiding this comment. I also think this should become an optional configuration.Hard encoding for 1000 may result in special needs can not be met There was a problem hiding this comment. @mfn : it is. Ini setting is bad, because relying on the user for such a crucial, misunderstood concept, is bad. |
||
|
|
||
| static zend_always_inline Bucket *zend_hash_find_bucket( | ||
| const HashTable *ht, zend_string *key, zend_bool for_insert) | ||
| { | ||
| zend_ulong h; | ||
| uint32_t nIndex; | ||
| uint32_t idx; | ||
| Bucket *p, *arData; | ||
| uint32_t num_collisions = 0; | ||
|
|
||
| h = zend_string_hash_val(key); | ||
| arData = ht->arData; | ||
|
|
@@ -481,15 +494,21 @@ static zend_always_inline Bucket *zend_hash_find_bucket(const HashTable *ht, zen | |
| return p; | ||
| } | ||
| idx = Z_NEXT(p->val); | ||
| num_collisions++; | ||
| } | ||
| if (for_insert && UNEXPECTED(num_collisions > HT_MAX_COLLISIONS)) { | ||
| zend_error_noreturn(E_ERROR, "Too many collisions in hashtable"); | ||
| } | ||
| return NULL; | ||
| } | ||
|
|
||
| static zend_always_inline Bucket *zend_hash_str_find_bucket( | ||
| const HashTable *ht, const char *str, size_t len, zend_ulong h, zend_bool for_insert) | ||
| { | ||
| uint32_t nIndex; | ||
| uint32_t idx; | ||
| Bucket *p, *arData; | ||
| uint32_t num_collisions = 0; | ||
|
|
||
| arData = ht->arData; | ||
| nIndex = h | ht->nTableMask; | ||
|
|
@@ -504,15 +523,21 @@ static zend_always_inline Bucket *zend_hash_str_find_bucket(const HashTable *ht, | |
| return p; | ||
| } | ||
| idx = Z_NEXT(p->val); | ||
| num_collisions++; | ||
| } | ||
| if (for_insert && UNEXPECTED(num_collisions > HT_MAX_COLLISIONS)) { | ||
| zend_error_noreturn(E_ERROR, "Too many collisions in hashtable"); | ||
| } | ||
| return NULL; | ||
| } | ||
|
|
||
| static zend_always_inline Bucket *zend_hash_index_find_bucket( | ||
| const HashTable *ht, zend_ulong h, zend_bool for_insert) | ||
| { | ||
| uint32_t nIndex; | ||
| uint32_t idx; | ||
| Bucket *p, *arData; | ||
| uint32_t num_collisions = 0; | ||
|
|
||
| arData = ht->arData; | ||
| nIndex = h | ht->nTableMask; | ||
|
|
@@ -524,6 +549,10 @@ static zend_always_inline Bucket *zend_hash_index_find_bucket(const HashTable *h | |
| return p; | ||
| } | ||
| idx = Z_NEXT(p->val); | ||
| num_collisions++; | ||
| } | ||
| if (for_insert && UNEXPECTED(num_collisions > HT_MAX_COLLISIONS)) { | ||
| zend_error_noreturn(E_ERROR, "Too many collisions in hashtable"); | ||
| } | ||
| return NULL; | ||
| } | ||
|
|
@@ -544,14 +573,17 @@ static zend_always_inline zval *_zend_hash_add_or_update_i(HashTable *ht, zend_s | |
| } else if (ht->u.flags & HASH_FLAG_PACKED) { | ||
| zend_hash_packed_to_hash(ht); | ||
| } else if ((flag & HASH_ADD_NEW) == 0) { | ||
| p = zend_hash_find_bucket(ht, key, 1); | ||
|
|
||
| if (p) { | ||
| zval *data; | ||
|
|
||
| if (flag & HASH_ADD) { | ||
| return NULL; | ||
| } | ||
| if (flag & HASH_ADD_OR_RETURN) { | ||
| return &p->val; | ||
| } | ||
| ZEND_ASSERT(&p->val != pData); | ||
| data = &p->val; | ||
| if ((flag & HASH_UPDATE_INDIRECT) && Z_TYPE_P(data) == IS_INDIRECT) { | ||
|
|
@@ -619,6 +651,11 @@ ZEND_API zval* ZEND_FASTCALL _zend_hash_add_new(HashTable *ht, zend_string *key, | |
| return _zend_hash_add_or_update_i(ht, key, pData, HASH_ADD_NEW ZEND_FILE_LINE_RELAY_CC); | ||
| } | ||
|
|
||
| ZEND_API zval* ZEND_FASTCALL _zend_hash_add_or_return(HashTable *ht, zend_string *key, zval *pData ZEND_FILE_LINE_DC) | ||
|
There was a problem hiding this comment. rename zend_hash_add_or_return() into zend_hash_lookup() |
||
| { | ||
| return _zend_hash_add_or_update_i(ht, key, pData, HASH_ADD_OR_RETURN ZEND_FILE_LINE_RELAY_CC); | ||
| } | ||
|
|
||
| ZEND_API zval* ZEND_FASTCALL _zend_hash_str_add_or_update(HashTable *ht, const char *str, size_t len, zval *pData, uint32_t flag ZEND_FILE_LINE_DC) | ||
| { | ||
| zend_string *key = zend_string_init(str, len, ht->u.flags & HASH_FLAG_PERSISTENT); | ||
|
|
@@ -706,6 +743,9 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, | |
| if (flag & HASH_ADD) { | ||
| return NULL; | ||
| } | ||
| if (flag & HASH_ADD_OR_RETURN) { | ||
| return &p->val; | ||
| } | ||
| if (ht->pDestructor) { | ||
| ht->pDestructor(&p->val); | ||
| } | ||
|
|
@@ -761,11 +801,14 @@ static zend_always_inline zval *_zend_hash_index_add_or_update_i(HashTable *ht, | |
| convert_to_hash: | ||
| zend_hash_packed_to_hash(ht); | ||
| } else if ((flag & HASH_ADD_NEW) == 0) { | ||
| p = zend_hash_index_find_bucket(ht, h, 1); | ||
| if (p) { | ||
| if (flag & HASH_ADD) { | ||
| return NULL; | ||
| } | ||
| if (flag & HASH_ADD_OR_RETURN) { | ||
| return &p->val; | ||
| } | ||
| ZEND_ASSERT(&p->val != pData); | ||
| HANDLE_BLOCK_INTERRUPTIONS(); | ||
| if (ht->pDestructor) { | ||
|
|
@@ -820,6 +863,11 @@ ZEND_API zval* ZEND_FASTCALL _zend_hash_index_add_new(HashTable *ht, zend_ulong | |
| return _zend_hash_index_add_or_update_i(ht, h, pData, HASH_ADD | HASH_ADD_NEW ZEND_FILE_LINE_RELAY_CC); | ||
| } | ||
|
|
||
| ZEND_API zval* ZEND_FASTCALL _zend_hash_index_add_or_return(HashTable *ht, zend_ulong h, zval *pData ZEND_FILE_LINE_DC) | ||
| { | ||
| return _zend_hash_index_add_or_update_i(ht, h, pData, HASH_ADD_OR_RETURN ZEND_FILE_LINE_RELAY_CC); | ||
| } | ||
|
|
||
| ZEND_API zval* ZEND_FASTCALL _zend_hash_index_update(HashTable *ht, zend_ulong h, zval *pData ZEND_FILE_LINE_DC) | ||
| { | ||
| return _zend_hash_index_add_or_update_i(ht, h, pData, HASH_UPDATE ZEND_FILE_LINE_RELAY_CC); | ||
|
|
@@ -1923,7 +1971,7 @@ ZEND_API zval* ZEND_FASTCALL zend_hash_find(const HashTable *ht, zend_string *ke | |
|
|
||
| IS_CONSISTENT(ht); | ||
|
|
||
| p = zend_hash_find_bucket(ht, key, 0); | ||
| return p ? &p->val : NULL; | ||
| } | ||
|
|
||
|
|
@@ -1935,7 +1983,7 @@ ZEND_API zval* ZEND_FASTCALL zend_hash_str_find(const HashTable *ht, const char | |
| IS_CONSISTENT(ht); | ||
|
|
||
| h = zend_inline_hash_func(str, len); | ||
| p = zend_hash_str_find_bucket(ht, str, len, h, 0); | ||
| return p ? &p->val : NULL; | ||
| } | ||
|
|
||
|
|
@@ -1945,7 +1993,7 @@ ZEND_API zend_bool ZEND_FASTCALL zend_hash_exists(const HashTable *ht, zend_stri | |
|
|
||
| IS_CONSISTENT(ht); | ||
|
|
||
| p = zend_hash_find_bucket(ht, key, 0); | ||
| return p ? 1 : 0; | ||
| } | ||
|
|
||
|
|
@@ -1957,7 +2005,7 @@ ZEND_API zend_bool ZEND_FASTCALL zend_hash_str_exists(const HashTable *ht, const | |
| IS_CONSISTENT(ht); | ||
|
|
||
| h = zend_inline_hash_func(str, len); | ||
| p = zend_hash_str_find_bucket(ht, str, len, h, 0); | ||
| return p ? 1 : 0; | ||
| } | ||
|
|
||
|
|
@@ -1977,11 +2025,10 @@ ZEND_API zval* ZEND_FASTCALL zend_hash_index_find(const HashTable *ht, zend_ulon | |
| return NULL; | ||
| } | ||
|
|
||
| p = zend_hash_index_find_bucket(ht, h, 0); | ||
| return p ? &p->val : NULL; | ||
| } | ||
|
|
||
| ZEND_API zend_bool ZEND_FASTCALL zend_hash_index_exists(const HashTable *ht, zend_ulong h) | ||
| { | ||
| Bucket *p; | ||
|
|
@@ -1997,7 +2044,7 @@ ZEND_API zend_bool ZEND_FASTCALL zend_hash_index_exists(const HashTable *ht, zen | |
| return 0; | ||
| } | ||
|
|
||
| p = zend_hash_index_find_bucket(ht, h, 0); | ||
| return p ? 1 : 0; | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should go to ext/json/tests and have skip if json ext is not loaded.