From: Nikita Popov Date: Wed, 10 Aug 2016 12:30:16 +0000 (+0200) Subject: Bug #72663 - part 2 X-Git-Tag: php-7.1.0beta3~33^2~5 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=61f2f5a0f760157f9c9d32d7d3df2be47a73e74d;p=php Bug #72663 - part 2 If a (nested) unserialize() call fails, we remove all the values that were inserted into var_hash during that call. This prevents their use in other unserializations in the same context. --- diff --git a/ext/standard/tests/serialize/bug72663_2.phpt b/ext/standard/tests/serialize/bug72663_2.phpt new file mode 100644 index 0000000000..8825dc5efc --- /dev/null +++ b/ext/standard/tests/serialize/bug72663_2.phpt @@ -0,0 +1,27 @@ +--TEST-- +Bug #72663 (2): Don't allow references into failed unserialize +--FILE-- +data); + } + function unserialize($data) { + $this->data = unserialize($data); + } +} + +$inner = 'a:1:{i:0;O:9:"Exception":2:{s:7:"'."\0".'*'."\0".'file";R:4;}'; +$exploit = 'a:2:{i:0;C:3:"obj":'.strlen($inner).':{'.$inner.'}i:1;R:4;}'; +var_dump(unserialize($exploit)); + +?> +--EXPECTF-- +Notice: unserialize(): Unexpected end of serialized data in %s on line %d + +Notice: unserialize(): Error at offset 46 of 47 bytes in %s on line %d + +Notice: unserialize(): Error at offset 79 of 80 bytes in %s on line %d +bool(false) diff --git a/ext/standard/var_unserializer.c b/ext/standard/var_unserializer.c index 317c3f9c48..6b33d84bd2 100644 --- a/ext/standard/var_unserializer.c +++ b/ext/standard/var_unserializer.c @@ -301,6 +301,8 @@ static inline size_t parse_uiv(const unsigned char *p) #define UNSERIALIZE_PARAMETER zval *rval, const unsigned char **p, const unsigned char *max, php_unserialize_data_t *var_hash, HashTable *classes #define UNSERIALIZE_PASSTHRU rval, p, max, var_hash, classes +static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER); + static zend_always_inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTable *ht, zend_long elements, int objprops) { while (elements-- > 0) { @@ -309,7 +311,7 @@ static zend_always_inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTab ZVAL_UNDEF(&key); - if (!php_var_unserialize_ex(&key, p, max, NULL, classes)) { + if (!php_var_unserialize_internal(&key, p, max, NULL, classes)) { zval_dtor(&key); return 0; } @@ -365,7 +367,7 @@ string_key: } } - if (!php_var_unserialize_ex(data, p, max, var_hash, classes)) { + if (!php_var_unserialize_internal(data, p, max, var_hash, classes)) { zval_dtor(&key); return 0; } @@ -502,8 +504,34 @@ PHPAPI int php_var_unserialize(zval *rval, const unsigned char **p, const unsign return php_var_unserialize_ex(UNSERIALIZE_PASSTHRU); } - PHPAPI int php_var_unserialize_ex(UNSERIALIZE_PARAMETER) +{ + var_entries *orig_var_entries = (*var_hash)->last; + zend_long orig_used_slots = orig_var_entries ? orig_var_entries->used_slots : 0; + int result; + + result = php_var_unserialize_internal(UNSERIALIZE_PASSTHRU); + + if (!result) { + /* If the unserialization failed, mark all elements that have been added to var_hash + * as NULL. This will forbid their use by other unserialize() calls in the same + * unserialization context. */ + var_entries *e = orig_var_entries; + zend_long s = orig_used_slots; + while (e) { + for (; s < e->used_slots; s++) { + e->data[s] = NULL; + } + + e = e->next; + s = 0; + } + } + + return result; +} + +static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER) { const unsigned char *cursor, *limit, *marker, *start; zval *rval_ref; @@ -522,7 +550,7 @@ PHPAPI int php_var_unserialize_ex(UNSERIALIZE_PARAMETER) start = cursor; -#line 526 "ext/standard/var_unserializer.c" +#line 554 "ext/standard/var_unserializer.c" { YYCTYPE yych; static const unsigned char yybm[] = { diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index fa8cce8880..9051894643 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -305,6 +305,8 @@ static inline size_t parse_uiv(const unsigned char *p) #define UNSERIALIZE_PARAMETER zval *rval, const unsigned char **p, const unsigned char *max, php_unserialize_data_t *var_hash, HashTable *classes #define UNSERIALIZE_PASSTHRU rval, p, max, var_hash, classes +static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER); + static zend_always_inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTable *ht, zend_long elements, int objprops) { while (elements-- > 0) { @@ -313,7 +315,7 @@ static zend_always_inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTab ZVAL_UNDEF(&key); - if (!php_var_unserialize_ex(&key, p, max, NULL, classes)) { + if (!php_var_unserialize_internal(&key, p, max, NULL, classes)) { zval_dtor(&key); return 0; } @@ -369,7 +371,7 @@ string_key: } } - if (!php_var_unserialize_ex(data, p, max, var_hash, classes)) { + if (!php_var_unserialize_internal(data, p, max, var_hash, classes)) { zval_dtor(&key); return 0; } @@ -506,8 +508,34 @@ PHPAPI int php_var_unserialize(zval *rval, const unsigned char **p, const unsign return php_var_unserialize_ex(UNSERIALIZE_PASSTHRU); } - PHPAPI int php_var_unserialize_ex(UNSERIALIZE_PARAMETER) +{ + var_entries *orig_var_entries = (*var_hash)->last; + zend_long orig_used_slots = orig_var_entries ? orig_var_entries->used_slots : 0; + int result; + + result = php_var_unserialize_internal(UNSERIALIZE_PASSTHRU); + + if (!result) { + /* If the unserialization failed, mark all elements that have been added to var_hash + * as NULL. This will forbid their use by other unserialize() calls in the same + * unserialization context. */ + var_entries *e = orig_var_entries; + zend_long s = orig_used_slots; + while (e) { + for (; s < e->used_slots; s++) { + e->data[s] = NULL; + } + + e = e->next; + s = 0; + } + } + + return result; +} + +static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER) { const unsigned char *cursor, *limit, *marker, *start; zval *rval_ref;