From 83eb4c8f46de445068802e8484d4167ca1763915 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 22 Aug 2019 17:12:33 +0200 Subject: [PATCH 1/4] Fix #78438: Corruption when __unserializing deeply nested structures When storing two temporary variables for delayed __unserialize() calls, we have to make sure that both fit into the same linked list element. We also remove `VAR_DTOR_ENTRIES_MAX` completely in favor of `VAR_ENTRIES_MAX`. --- ext/standard/tests/serialize/bug78438.phpt | 121 +++++++++++++++++++++ ext/standard/var_unserializer.re | 13 +-- 2 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 ext/standard/tests/serialize/bug78438.phpt diff --git a/ext/standard/tests/serialize/bug78438.phpt b/ext/standard/tests/serialize/bug78438.phpt new file mode 100644 index 0000000000000..633f4619217cc --- /dev/null +++ b/ext/standard/tests/serialize/bug78438.phpt @@ -0,0 +1,121 @@ +--TEST-- +Bug #78438 (Corruption when __unserializing deeply nested structures) +--FILE-- +childs]; + } + + public function __unserialize(array $data) + { + list($this->childs) = $data; + } +} + +function createTree ($width, $depth) { + $root = new Node(); + + $nextLevel = [$root]; + + for ($level=1; $level<$depth; $level++) { + $levelRoots = $nextLevel; + $nextLevel = []; + + while (count($levelRoots) > 0) { + $levelRoot = array_shift($levelRoots); + + for ($w = 0; $w < $width; $w++) { + $tester = new Node(); + + $levelRoot->childs[] = $tester; + + $nextLevel[] = $tester; + } + } + } + + return $root; +} + + +$width = 3; +ob_implicit_flush(); + +foreach (range(1, 8) as $depth) { + $tree = createTree($width, $depth); + + echo "Testcase tree $width x $depth".PHP_EOL; + + echo "> Serializing now".PHP_EOL; + $serialized = serialize($tree); + + echo "> Unserializing now".PHP_EOL; + $tree = unserialize($serialized); + + // Lets test whether all is ok! + $expectedSize = ($width**$depth - 1)/($width-1); + + $nodes = [$tree]; + $count = 0; + + while (count($nodes) > 0) { + $count++; + + $node = array_shift($nodes); + foreach ($node->childs as $node) { + $nodes[] = $node; + } + } + + echo "> Unserialized total node count was $count, expected $expectedSize: ".($expectedSize === $count ? 'CORRECT!' : 'INCORRECT'); + + echo PHP_EOL; + echo PHP_EOL; +} +?> +--EXPECT-- +Testcase tree 3 x 1 +> Serializing now +> Unserializing now +> Unserialized total node count was 1, expected 1: CORRECT! + +Testcase tree 3 x 2 +> Serializing now +> Unserializing now +> Unserialized total node count was 4, expected 4: CORRECT! + +Testcase tree 3 x 3 +> Serializing now +> Unserializing now +> Unserialized total node count was 13, expected 13: CORRECT! + +Testcase tree 3 x 4 +> Serializing now +> Unserializing now +> Unserialized total node count was 40, expected 40: CORRECT! + +Testcase tree 3 x 5 +> Serializing now +> Unserializing now +> Unserialized total node count was 121, expected 121: CORRECT! + +Testcase tree 3 x 6 +> Serializing now +> Unserializing now +> Unserialized total node count was 364, expected 364: CORRECT! + +Testcase tree 3 x 7 +> Serializing now +> Unserializing now +> Unserialized total node count was 1093, expected 1093: CORRECT! + +Testcase tree 3 x 8 +> Serializing now +> Unserializing now +> Unserialized total node count was 3280, expected 3280: CORRECT! diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index 71133e239c647..feaa515729f91 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -23,7 +23,6 @@ /* {{{ reference-handling for unserializer: var_* */ #define VAR_ENTRIES_MAX 1018 /* 1024 - offsetof(php_unserialize_data, entries) / sizeof(void*) */ -#define VAR_DTOR_ENTRIES_MAX 255 /* 256 - offsetof(var_dtor_entries, data) / sizeof(zval) */ #define VAR_ENTRIES_DBG 0 /* VAR_FLAG used in var_dtor entries to signify an entry on which @@ -114,7 +113,7 @@ static inline void var_push(php_unserialize_data_t *var_hashx, zval *rval) PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval *rval) { if (Z_REFCOUNTED_P(rval)) { - zval *tmp_var = var_tmp_var(var_hashx); + zval *tmp_var = var_tmp_var(var_hashx, 0); if (!tmp_var) { return; } @@ -122,7 +121,7 @@ PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval *rval) } } -PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx) +PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx, int one_more) { var_dtor_entries *var_hash; @@ -131,7 +130,7 @@ PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx) } var_hash = (*var_hashx)->last_dtor; - if (!var_hash || var_hash->used_slots == VAR_DTOR_ENTRIES_MAX) { + if (!var_hash || var_hash->used_slots + one_more >= VAR_ENTRIES_MAX) { var_hash = emalloc(sizeof(var_dtor_entries)); var_hash->used_slots = 0; var_hash->next = 0; @@ -653,10 +652,10 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, zend_ /* Delay __unserialize() call until end of serialization. We use two slots here to * store both the object and the unserialized data array. */ ZVAL_DEREF(rval); - tmp = var_tmp_var(var_hash); + tmp = var_tmp_var(var_hash, 1); ZVAL_COPY(tmp, rval); Z_EXTRA_P(tmp) = VAR_UNSERIALIZE_FLAG; - tmp = var_tmp_var(var_hash); + tmp = var_tmp_var(var_hash, 0); ZVAL_COPY_VALUE(tmp, &ary); return finish_nested_data(UNSERIALIZE_PASSTHRU); @@ -682,7 +681,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, zend_ ZVAL_DEREF(rval); if (has_wakeup) { /* Delay __wakeup call until end of serialization */ - zval *wakeup_var = var_tmp_var(var_hash); + zval *wakeup_var = var_tmp_var(var_hash, 0); ZVAL_COPY(wakeup_var, rval); Z_EXTRA_P(wakeup_var) = VAR_WAKEUP_FLAG; } From 60d79d84e89a4f0f41381c6c3d86f6c6ca90cd36 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 22 Aug 2019 18:13:17 +0200 Subject: [PATCH 2/4] Avoid API break --- ext/standard/var_unserializer.re | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index feaa515729f91..57397def54ed6 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -113,7 +113,7 @@ static inline void var_push(php_unserialize_data_t *var_hashx, zval *rval) PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval *rval) { if (Z_REFCOUNTED_P(rval)) { - zval *tmp_var = var_tmp_var(var_hashx, 0); + zval *tmp_var = var_tmp_var(var_hashx); if (!tmp_var) { return; } @@ -121,7 +121,7 @@ PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval *rval) } } -PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx, int one_more) +static zend_always_inline zval *tmp_var(php_unserialize_data_t *var_hashx, int extra) { var_dtor_entries *var_hash; @@ -130,7 +130,7 @@ PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx, int one_more) } var_hash = (*var_hashx)->last_dtor; - if (!var_hash || var_hash->used_slots + one_more >= VAR_ENTRIES_MAX) { + if (!var_hash || var_hash->used_slots + extra >= VAR_ENTRIES_MAX) { var_hash = emalloc(sizeof(var_dtor_entries)); var_hash->used_slots = 0; var_hash->next = 0; @@ -148,6 +148,11 @@ PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx, int one_more) return &var_hash->data[var_hash->used_slots++]; } +PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx) +{ + return tmp_var(var_hashx, 0); +} + PHPAPI void var_replace(php_unserialize_data_t *var_hashx, zval *ozval, zval *nzval) { zend_long i; @@ -652,10 +657,10 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, zend_ /* Delay __unserialize() call until end of serialization. We use two slots here to * store both the object and the unserialized data array. */ ZVAL_DEREF(rval); - tmp = var_tmp_var(var_hash, 1); + tmp = tmp_var(var_hash, 1); ZVAL_COPY(tmp, rval); Z_EXTRA_P(tmp) = VAR_UNSERIALIZE_FLAG; - tmp = var_tmp_var(var_hash, 0); + tmp = tmp_var(var_hash, 0); ZVAL_COPY_VALUE(tmp, &ary); return finish_nested_data(UNSERIALIZE_PASSTHRU); @@ -681,7 +686,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, zend_ ZVAL_DEREF(rval); if (has_wakeup) { /* Delay __wakeup call until end of serialization */ - zval *wakeup_var = var_tmp_var(var_hash, 0); + zval *wakeup_var = var_tmp_var(var_hash); ZVAL_COPY(wakeup_var, rval); Z_EXTRA_P(wakeup_var) = VAR_WAKEUP_FLAG; } From 2f1d2bd123eb80bc1a62f2c04d0521ed9c3f4107 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 22 Aug 2019 23:40:32 +0200 Subject: [PATCH 3/4] Restore `VAR_DTOR_ENTRIES_MAX` --- ext/standard/var_unserializer.re | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index 57397def54ed6..80aea89d1f224 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -23,6 +23,7 @@ /* {{{ reference-handling for unserializer: var_* */ #define VAR_ENTRIES_MAX 1018 /* 1024 - offsetof(php_unserialize_data, entries) / sizeof(void*) */ +#define VAR_DTOR_ENTRIES_MAX 255 /* 256 - offsetof(var_dtor_entries, data) / sizeof(zval) */ #define VAR_ENTRIES_DBG 0 /* VAR_FLAG used in var_dtor entries to signify an entry on which @@ -39,7 +40,7 @@ typedef struct { typedef struct { zend_long used_slots; void *next; - zval data[VAR_ENTRIES_MAX]; + zval data[VAR_DTOR_ENTRIES_MAX]; } var_dtor_entries; struct php_unserialize_data { @@ -130,7 +131,7 @@ static zend_always_inline zval *tmp_var(php_unserialize_data_t *var_hashx, int e } var_hash = (*var_hashx)->last_dtor; - if (!var_hash || var_hash->used_slots + extra >= VAR_ENTRIES_MAX) { + if (!var_hash || var_hash->used_slots + extra >= VAR_DTOR_ENTRIES_MAX) { var_hash = emalloc(sizeof(var_dtor_entries)); var_hash->used_slots = 0; var_hash->next = 0; From 65a6b919ea5b031f74177db1d6b7c4038e921402 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 23 Aug 2019 00:25:44 +0200 Subject: [PATCH 4/4] `tmp_var` actually reserves `num` slots --- ext/standard/var_unserializer.re | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index 80aea89d1f224..cef2eb064081c 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -122,16 +122,17 @@ PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval *rval) } } -static zend_always_inline zval *tmp_var(php_unserialize_data_t *var_hashx, int extra) +static zend_always_inline zval *tmp_var(php_unserialize_data_t *var_hashx, zend_long num) { var_dtor_entries *var_hash; + zend_long used_slots; - if (!var_hashx || !*var_hashx) { + if (!var_hashx || !*var_hashx || num < 1) { return NULL; } var_hash = (*var_hashx)->last_dtor; - if (!var_hash || var_hash->used_slots + extra >= VAR_DTOR_ENTRIES_MAX) { + if (!var_hash || var_hash->used_slots + num > VAR_DTOR_ENTRIES_MAX) { var_hash = emalloc(sizeof(var_dtor_entries)); var_hash->used_slots = 0; var_hash->next = 0; @@ -144,14 +145,16 @@ static zend_always_inline zval *tmp_var(php_unserialize_data_t *var_hashx, int e (*var_hashx)->last_dtor = var_hash; } - ZVAL_UNDEF(&var_hash->data[var_hash->used_slots]); - Z_EXTRA(var_hash->data[var_hash->used_slots]) = 0; - return &var_hash->data[var_hash->used_slots++]; + for (used_slots = var_hash->used_slots; var_hash->used_slots < used_slots + num; var_hash->used_slots++) { + ZVAL_UNDEF(&var_hash->data[var_hash->used_slots]); + Z_EXTRA(var_hash->data[var_hash->used_slots]) = 0; + } + return &var_hash->data[used_slots]; } PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx) { - return tmp_var(var_hashx, 0); + return tmp_var(var_hashx, 1); } PHPAPI void var_replace(php_unserialize_data_t *var_hashx, zval *ozval, zval *nzval) @@ -658,10 +661,10 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, zend_ /* Delay __unserialize() call until end of serialization. We use two slots here to * store both the object and the unserialized data array. */ ZVAL_DEREF(rval); - tmp = tmp_var(var_hash, 1); + tmp = tmp_var(var_hash, 2); ZVAL_COPY(tmp, rval); Z_EXTRA_P(tmp) = VAR_UNSERIALIZE_FLAG; - tmp = tmp_var(var_hash, 0); + tmp++; ZVAL_COPY_VALUE(tmp, &ary); return finish_nested_data(UNSERIALIZE_PASSTHRU);