Fixed #74699 - Broken ArrayIterator unserializing

Author: andrewnester

NEWS

@@ -2,7 +2,8 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? 2017 PHP 7.0.23
 
-
+- SPL:
+  . Fixed bug #74669 (Unserialize ArrayIterator broken). (Andrew Nester)
 
 03 Aug 2017 PHP 7.0.22
 

ext/spl/spl_array.c

@@ -1734,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) {
@@ -1782,24 +1783,38 @@ SPL_METHOD(Array, unserialize)
 	}
 	++p;
 
-	if (*p!='m') {
+	if (flags & SPL_ARRAY_IS_SELF) {
+		/* If IS_SELF is used, the flags are not followed by an array/object */
+		intern->ar_flags &= ~SPL_ARRAY_CLONE_MASK;
+		intern->ar_flags |= flags & SPL_ARRAY_CLONE_MASK;
+		zval_ptr_dtor(&intern->array);
+		ZVAL_UNDEF(&intern->array);
+	} else {
 		if (*p!='a' && *p!='O' && *p!='C' && *p!='r') {
 			goto outexcept;
 		}
+
+		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;
+		}
+
 		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 (Z_TYPE_P(array) == IS_ARRAY) {
+			zval_ptr_dtor(&intern->array);
+			ZVAL_COPY(&intern->array, array);
+		} else {
+			spl_array_set_array(object, intern, array, 0L, 1);
+		}
+
+		if (*p != ';') {
 			goto outexcept;
 		}
-		var_push_dtor(&var_hash, &intern->array);
-	}
-	if (*p != ';') {
-		goto outexcept;
+        ++p;
 	}
-	++p;
 
 	/* members */
 	if (*p!= 'm' || *++p != ':') {

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

ext/spl/tests/bug74669.phpt

@@ -0,0 +1,112 @@
+--TEST--
+Bug #74669: Unserialize ArrayIterator broken
+--FILE--
+<?php
+
+class Container implements Iterator
+{
+    public $container;
+    public $iterator;
+
+    public function __construct()
+    {
+        $this->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();
+    }
+}
+
+class SelfArray extends ArrayObject
+{
+    public function __construct()
+    {
+        parent::__construct($this);
+    }
+}
+
+$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;
+}
+
+$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']);
+
+$selfArray = new SelfArray();
+$selfArray['foo'] = 'bar';
+var_dump($selfArray);
+$serialized = serialize($selfArray);
+var_dump($serialized);
+unset($selfArray);
+$selfArray = unserialize($serialized);
+var_dump($selfArray);
+var_dump($selfArray['foo']);
+
+?>
+--EXPECTF--
+0 => test1
+1 => test2
+0 => test1
+1 => test2
+
+Notice: Undefined index: foo in %s on line %s
+NULL
+object(SelfArray)#9 (1) {
+  ["foo"]=>
+  string(3) "bar"
+}
+string(62) "C:9:"SelfArray":41:{x:i:16777216;m:a:1:{s:3:"foo";s:3:"bar";}}"
+object(SelfArray)#9 (1) {
+  ["foo"]=>
+  string(3) "bar"
+}
+string(3) "bar"