From cdac509caf87cc7c49061c8e6edd23ccd70a0c40 Mon Sep 17 00:00:00 2001 From: andrewnester Date: Tue, 6 Jun 2017 09:37:05 +0300 Subject: [PATCH 1/3] Fixed #74699 - Broken ArrayIterator unserializing --- ext/spl/spl_array.c | 7 +-- ext/spl/tests/array_025.phpt | 2 +- ext/spl/tests/bug74669.phpt | 82 ++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 ext/spl/tests/bug74669.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 556878f0e1d15..41a7977dff196 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -56,6 +56,7 @@ PHPAPI zend_class_entry *spl_ce_RecursiveArrayIterator; #define SPL_ARRAY_USE_OTHER 0x02000000 #define SPL_ARRAY_INT_MASK 0xFFFF0000 #define SPL_ARRAY_CLONE_MASK 0x0100FFFF +#define SPL_ARRAY_SERIALIZE_MASK 0x0200FFFF #define SPL_ARRAY_METHOD_NO_ARG 0 #define SPL_ARRAY_METHOD_USE_ARG 1 @@ -1698,7 +1699,7 @@ SPL_METHOD(Array, serialize) PHP_VAR_SERIALIZE_INIT(var_hash); - ZVAL_LONG(&flags, (intern->ar_flags & SPL_ARRAY_CLONE_MASK)); + ZVAL_LONG(&flags, (intern->ar_flags & SPL_ARRAY_SERIALIZE_MASK)); /* storage */ smart_str_appendl(&buf, "x:", 2); @@ -1786,8 +1787,8 @@ SPL_METHOD(Array, unserialize) if (*p!='a' && *p!='O' && *p!='C' && *p!='r') { goto outexcept; } - intern->ar_flags &= ~SPL_ARRAY_CLONE_MASK; - intern->ar_flags |= flags & SPL_ARRAY_CLONE_MASK; + intern->ar_flags &= ~SPL_ARRAY_SERIALIZE_MASK; + intern->ar_flags |= flags & SPL_ARRAY_SERIALIZE_MASK; zval_ptr_dtor(&intern->array); ZVAL_UNDEF(&intern->array); if (!php_var_unserialize(&intern->array, &p, s + buf_len, &var_hash) diff --git a/ext/spl/tests/array_025.phpt b/ext/spl/tests/array_025.phpt index 35893ea1eaf55..5ee74eaa96512 100644 --- a/ext/spl/tests/array_025.phpt +++ b/ext/spl/tests/array_025.phpt @@ -24,7 +24,7 @@ ArrayObject Object ) ) -C:11:"ArrayObject":76:{x:i:0;C:11:"ArrayObject":37:{x:i:0;a:2:{i:0;i:1;i:1;i:2;};m:a:0:{}};m:a:0:{}} +C:11:"ArrayObject":83:{x:i:33554432;C:11:"ArrayObject":37:{x:i:0;a:2:{i:0;i:1;i:1;i:2;};m:a:0:{}};m:a:0:{}} ArrayObject Object ( [storage:ArrayObject:private] => ArrayObject Object diff --git a/ext/spl/tests/bug74669.phpt b/ext/spl/tests/bug74669.phpt new file mode 100644 index 0000000000000..752902e936c00 --- /dev/null +++ b/ext/spl/tests/bug74669.phpt @@ -0,0 +1,82 @@ +--TEST-- +Bug #74669: Unserialize ArrayIterator broken +--FILE-- +container = new ArrayObject(); + $this->iterator = $this->container->getIterator(); + } + + public function append($element) + { + $this->container->append($element); + } + + public function current() + { + return $this->iterator->current(); + } + + public function next() + { + $this->iterator->next(); + } + + public function key() + { + return $this->iterator->key(); + } + + public function valid() + { + return $this->iterator->valid(); + } + + public function rewind() + { + $this->iterator->rewind(); + } +} + +$container = new Container(); +$container->append('test1'); +$container->append('test2'); +$container->valid(); +$serialized = serialize($container); +unset($container); + +$container = unserialize($serialized); + +foreach ($container as $key => $value) { + echo $key . ' => ' . $value . PHP_EOL; +} + +// test to check unserialize of old format +$oldFormat = 'C:11:"ArrayObject":76:{x:i:0;C:11:"ArrayObject":37:{x:i:0;a:2:{i:0;i:1;i:1;i:2;};m:a:0:{}};m:a:0:{}}'; +$obj = unserialize($oldFormat); +print_r($obj); + +?> +--EXPECT-- +0 => test1 +1 => test2 +ArrayObject Object +( + [storage:ArrayObject:private] => ArrayObject Object + ( + [storage:ArrayObject:private] => Array + ( + [0] => 1 + [1] => 2 + ) + + ) + +) From 823d637469a8ed1119bac55ce704280dd3b24e2d Mon Sep 17 00:00:00 2001 From: andrewnester Date: Tue, 13 Jun 2017 10:18:20 +0300 Subject: [PATCH 2/3] Patch reworked to use spl_array_set_array(). --- ext/spl/spl_array.c | 25 ++++++++++++++++-------- ext/spl/tests/array_025.phpt | 2 +- ext/spl/tests/bug74669.phpt | 38 +++++++++++++++++++----------------- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 41a7977dff196..c973452c4e47a 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -56,7 +56,6 @@ PHPAPI zend_class_entry *spl_ce_RecursiveArrayIterator; #define SPL_ARRAY_USE_OTHER 0x02000000 #define SPL_ARRAY_INT_MASK 0xFFFF0000 #define SPL_ARRAY_CLONE_MASK 0x0100FFFF -#define SPL_ARRAY_SERIALIZE_MASK 0x0200FFFF #define SPL_ARRAY_METHOD_NO_ARG 0 #define SPL_ARRAY_METHOD_USE_ARG 1 @@ -1699,7 +1698,7 @@ SPL_METHOD(Array, serialize) PHP_VAR_SERIALIZE_INIT(var_hash); - ZVAL_LONG(&flags, (intern->ar_flags & SPL_ARRAY_SERIALIZE_MASK)); + ZVAL_LONG(&flags, (intern->ar_flags & SPL_ARRAY_CLONE_MASK)); /* storage */ smart_str_appendl(&buf, "x:", 2); @@ -1735,13 +1734,14 @@ SPL_METHOD(Array, serialize) */ SPL_METHOD(Array, unserialize) { - spl_array_object *intern = Z_SPLARRAY_P(getThis()); + zval *object = getThis(); + spl_array_object *intern = Z_SPLARRAY_P(object); char *buf; size_t buf_len; const unsigned char *p, *s; php_unserialize_data_t var_hash; - zval *members, *zflags; + zval *members, *zflags, array; zend_long flags; if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &buf, &buf_len) == FAILURE) { @@ -1787,14 +1787,23 @@ SPL_METHOD(Array, unserialize) if (*p!='a' && *p!='O' && *p!='C' && *p!='r') { goto outexcept; } - intern->ar_flags &= ~SPL_ARRAY_SERIALIZE_MASK; - intern->ar_flags |= flags & SPL_ARRAY_SERIALIZE_MASK; + intern->ar_flags &= ~SPL_ARRAY_CLONE_MASK; + intern->ar_flags |= flags & SPL_ARRAY_CLONE_MASK; zval_ptr_dtor(&intern->array); ZVAL_UNDEF(&intern->array); - if (!php_var_unserialize(&intern->array, &p, s + buf_len, &var_hash) - || (Z_TYPE(intern->array) != IS_ARRAY && Z_TYPE(intern->array) != IS_OBJECT)) { + + if (!php_var_unserialize(&array, &p, s + buf_len, &var_hash) + || (Z_TYPE(array) != IS_ARRAY && Z_TYPE(array) != IS_OBJECT)) { goto outexcept; } + + if (Z_TYPE(array) == IS_ARRAY || + Z_OBJ_HT(array) == &spl_handler_ArrayObject || + Z_OBJ_HT(array) == &spl_handler_ArrayIterator) { + spl_array_set_array(object, intern, &array, 0L, 1); + } else { + ZVAL_COPY(&intern->array, &array); + } var_push_dtor(&var_hash, &intern->array); } if (*p != ';') { diff --git a/ext/spl/tests/array_025.phpt b/ext/spl/tests/array_025.phpt index 5ee74eaa96512..35893ea1eaf55 100644 --- a/ext/spl/tests/array_025.phpt +++ b/ext/spl/tests/array_025.phpt @@ -24,7 +24,7 @@ ArrayObject Object ) ) -C:11:"ArrayObject":83:{x:i:33554432;C:11:"ArrayObject":37:{x:i:0;a:2:{i:0;i:1;i:1;i:2;};m:a:0:{}};m:a:0:{}} +C:11:"ArrayObject":76:{x:i:0;C:11:"ArrayObject":37:{x:i:0;a:2:{i:0;i:1;i:1;i:2;};m:a:0:{}};m:a:0:{}} ArrayObject Object ( [storage:ArrayObject:private] => ArrayObject Object diff --git a/ext/spl/tests/bug74669.phpt b/ext/spl/tests/bug74669.phpt index 752902e936c00..dad62739e4e49 100644 --- a/ext/spl/tests/bug74669.phpt +++ b/ext/spl/tests/bug74669.phpt @@ -58,25 +58,27 @@ foreach ($container as $key => $value) { echo $key . ' => ' . $value . PHP_EOL; } -// test to check unserialize of old format -$oldFormat = 'C:11:"ArrayObject":76:{x:i:0;C:11:"ArrayObject":37:{x:i:0;a:2:{i:0;i:1;i:1;i:2;};m:a:0:{}};m:a:0:{}}'; -$obj = unserialize($oldFormat); -print_r($obj); +$arObj = new ArrayObject(['test1', 'test2']); +$serialized = serialize($container); +unset($arObj); + +$arObj = unserialize($serialized); +foreach($arObj as $key => $value) { + echo $key . ' => ' . $value . PHP_EOL; +} + +$payload = 'x:i:33554432;O:8:"stdClass":0:{};m:a:0:{}'; +$str = 'C:11:"ArrayObject":' . strlen($payload) . ':{' . $payload . '}'; +$ao = unserialize($str); +var_dump($ao['foo']); ?> ---EXPECT-- +--EXPECTF-- 0 => test1 1 => test2 -ArrayObject Object -( - [storage:ArrayObject:private] => ArrayObject Object - ( - [storage:ArrayObject:private] => Array - ( - [0] => 1 - [1] => 2 - ) - - ) - -) +0 => test1 +1 => test2 + +Notice: Undefined index: foo in %s on line %s +NULL + From 8c429184f8a295024f9c65ffc38b6f4554e3fd83 Mon Sep 17 00:00:00 2001 From: andrewnester Date: Fri, 30 Jun 2017 17:23:48 +0300 Subject: [PATCH 3/3] Changes to make it work with IS_SELF array objects. --- ext/spl/spl_array.c | 26 ++++++++++----------- ext/spl/tests/bug70155.phpt | 45 +++++-------------------------------- ext/spl/tests/bug74669.phpt | 20 +++++++++++++++++ 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index c973452c4e47a..d9a7e4273f619 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -1741,7 +1741,7 @@ SPL_METHOD(Array, unserialize) size_t buf_len; const unsigned char *p, *s; php_unserialize_data_t var_hash; - zval *members, *zflags, array; + zval *members, *zflags, *array; zend_long flags; if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &buf, &buf_len) == FAILURE) { @@ -1792,24 +1792,22 @@ SPL_METHOD(Array, unserialize) zval_ptr_dtor(&intern->array); ZVAL_UNDEF(&intern->array); - if (!php_var_unserialize(&array, &p, s + buf_len, &var_hash) - || (Z_TYPE(array) != IS_ARRAY && Z_TYPE(array) != IS_OBJECT)) { + array = var_tmp_var(&var_hash); + if (!php_var_unserialize(array, &p, s + buf_len, &var_hash) + || (Z_TYPE_P(array) != IS_ARRAY && Z_TYPE_P(array) != IS_OBJECT)) { goto outexcept; } - - if (Z_TYPE(array) == IS_ARRAY || - Z_OBJ_HT(array) == &spl_handler_ArrayObject || - Z_OBJ_HT(array) == &spl_handler_ArrayIterator) { - spl_array_set_array(object, intern, &array, 0L, 1); + if (Z_TYPE_P(array) == IS_ARRAY) { + ZVAL_COPY(&intern->array, array); } else { - ZVAL_COPY(&intern->array, &array); + spl_array_set_array(object, intern, array, 0L, 1); } - var_push_dtor(&var_hash, &intern->array); - } - if (*p != ';') { - goto outexcept; + + if (*p != ';') { + goto outexcept; + } + ++p; } - ++p; /* members */ if (*p!= 'm' || *++p != ':') { diff --git a/ext/spl/tests/bug70155.phpt b/ext/spl/tests/bug70155.phpt index 1730a1a587f91..0aa246cc2388f 100644 --- a/ext/spl/tests/bug70155.phpt +++ b/ext/spl/tests/bug70155.phpt @@ -8,43 +8,10 @@ $data = unserialize($exploit); var_dump($data); ?> -===DONE=== --EXPECTF-- -object(ArrayObject)#1 (2) { - [0]=> - int(0) - ["storage":"ArrayObject":private]=> - object(DateInterval)#2 (15) { - ["y"]=> - int(3) - ["m"]=> - int(-1) - ["d"]=> - int(-1) - ["h"]=> - int(-1) - ["i"]=> - int(-1) - ["s"]=> - int(-1) - ["weekday"]=> - int(-1) - ["weekday_behavior"]=> - int(-1) - ["first_last_day_of"]=> - int(-1) - ["invert"]=> - int(0) - ["days"]=> - int(-1) - ["special_type"]=> - int(0) - ["special_amount"]=> - int(-1) - ["have_weekday_relative"]=> - int(0) - ["have_special_relative"]=> - int(0) - } -} -===DONE=== +Fatal error: Uncaught InvalidArgumentException: Overloaded object of type DateInterval is not compatible with ArrayObject in %s +Stack trace: +%s +%s +%s +%s diff --git a/ext/spl/tests/bug74669.phpt b/ext/spl/tests/bug74669.phpt index dad62739e4e49..211461737cf5c 100644 --- a/ext/spl/tests/bug74669.phpt +++ b/ext/spl/tests/bug74669.phpt @@ -45,6 +45,14 @@ class Container implements Iterator } } +class SelfArray extends ArrayObject +{ + public function __construct() + { + parent::__construct($this); + } +} + $container = new Container(); $container->append('test1'); $container->append('test2'); @@ -69,9 +77,16 @@ foreach($arObj as $key => $value) { $payload = 'x:i:33554432;O:8:"stdClass":0:{};m:a:0:{}'; $str = 'C:11:"ArrayObject":' . strlen($payload) . ':{' . $payload . '}'; + $ao = unserialize($str); var_dump($ao['foo']); +$selfArray = new SelfArray(); +$serialized = serialize($selfArray); +unset($selfArray); +$selfArray = unserialize($serialized); +var_dump($selfArray); + ?> --EXPECTF-- 0 => test1 @@ -81,4 +96,9 @@ var_dump($ao['foo']); Notice: Undefined index: foo in %s on line %s NULL +object(SelfArray)#9 (1) { + ["storage":"ArrayObject":private]=> + array(0) { + } +}