Fix use-after-free in ArrayObject::unset() with destructor

Zend/tests/gh16646.phpt

@@ -0,0 +1,32 @@
+--TEST--
+GH-16646: Use-after-free in ArrayObject::unset() with destructor
+--FILE--
+<?php
+
+class B {
+    public $b;
+    function __construct($arg) {
+        $this->b = $arg;
+    }
+}
+
+class C {
+    function __destruct() {
+        global $arr;
+        echo __METHOD__, "\n";
+        $arr->exchangeArray([]);
+    }
+}
+
+$arr = new ArrayObject(new B(new C));
+unset($arr["b"]);
+var_dump($arr);
+
+?>
+--EXPECT--
+C::__destruct
+object(ArrayObject)#1 (1) {
+  ["storage":"ArrayObject":private]=>
+  array(0) {
+  }
+}

Zend/tests/gh16646_2.phpt

@@ -0,0 +1,25 @@
+--TEST--
+GH-16646: Use-after-free in ArrayObject::exchangeArray() with destructor
+--FILE--
+<?php
+
+class C {
+    function __destruct() {
+        global $arr;
+        echo __METHOD__, "\n";
+        $arr->exchangeArray([]);
+    }
+}
+
+$arr = new ArrayObject(new C);
+$arr->exchangeArray([]);
+var_dump($arr);
+
+?>
+--EXPECT--
+C::__destruct
+object(ArrayObject)#1 (1) {
+  ["storage":"ArrayObject":private]=>
+  array(0) {
+  }
+}

ext/spl/spl_array.c

@@ -555,13 +555,15 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
 			if (Z_TYPE_P(data) == IS_INDIRECT) {
 				data = Z_INDIRECT_P(data);
 				if (Z_TYPE_P(data) != IS_UNDEF) {
-					zval_ptr_dtor(data);
+					zval garbage;
+					ZVAL_COPY_VALUE(&garbage, data);
 					ZVAL_UNDEF(data);
 					HT_FLAGS(ht) |= HASH_FLAG_HAS_EMPTY_IND;
 					zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern));
 					if (spl_array_is_object(intern)) {
 						spl_array_skip_protected(intern, ht);
 					}
+					zval_ptr_dtor(&garbage);
 				}
 			} else {
 				zend_hash_del(ht, key.key);
@@ -1052,8 +1054,10 @@ static HashTable *spl_array_it_get_gc(zend_object_iterator *iter, zval **table,
 static void spl_array_set_array(zval *object, spl_array_object *intern, zval *array, zend_long ar_flags, bool just_array) {
 	/* Handled by ZPP prior to this, or for __unserialize() before passing to here */
 	ZEND_ASSERT(Z_TYPE_P(array) == IS_ARRAY || Z_TYPE_P(array) == IS_OBJECT);
+	zval garbage;
+	ZVAL_UNDEF(&garbage);
 	if (Z_TYPE_P(array) == IS_ARRAY) {
-		zval_ptr_dtor(&intern->array);
+		ZVAL_COPY_VALUE(&garbage, &intern->array);
 		if (Z_REFCOUNT_P(array) == 1) {
 			ZVAL_COPY(&intern->array, array);
 		} else {
@@ -1071,7 +1075,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
 		}
 	} else {
 		if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) {
-			zval_ptr_dtor(&intern->array);
+			ZVAL_COPY_VALUE(&garbage, &intern->array);
 			if (just_array)	{
 				spl_array_object *other = Z_SPLARRAY_P(array);
 				ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK;
@@ -1089,9 +1093,10 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
 				zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0,
 					"Overloaded object of type %s is not compatible with %s",
 					ZSTR_VAL(Z_OBJCE_P(array)->name), ZSTR_VAL(intern->std.ce->name));
+				ZEND_ASSERT(Z_TYPE(garbage) == IS_UNDEF);
 				return;
 			}
-			zval_ptr_dtor(&intern->array);
+			ZVAL_COPY_VALUE(&garbage, &intern->array);
 			ZVAL_COPY(&intern->array, array);
 		}
 	}
@@ -1102,6 +1107,8 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
 		zend_hash_iterator_del(intern->ht_iter);
 		intern->ht_iter = (uint32_t)-1;
 	}
+
+	zval_ptr_dtor(&garbage);
 }
 /* }}} */
 

ext/spl/tests/gh16646.phpt

@@ -0,0 +1,32 @@
+--TEST--
+GH-16646: Test
+--FILE--
+<?php
+
+class B {
+    public $b;
+    function __construct($arg) {
+        $this->b = $arg;
+    }
+}
+
+class C {
+    function __destruct() {
+        global $arr;
+        echo __METHOD__, "\n";
+        $arr->exchangeArray([]);
+    }
+}
+
+$arr = new ArrayObject(new B(new C));
+unset($arr["b"]);
+var_dump($arr);
+
+?>
+--EXPECT--
+C::__destruct
+object(ArrayObject)#1 (1) {
+  ["storage":"ArrayObject":private]=>
+  array(0) {
+  }
+}