Merge branch 'PHP-8.3' into PHP-8.4

* PHP-8.3: Add NEWS entry Also fix same issue in ArrayObject::exchangeArray() Fix use-after-free in ArrayObject::unset() with destructor
iluuu1994 · Nov 4, 2024 · 66ad4ce · 66ad4ce
2 parents 085c151 + dca438e
commit 66ad4ce
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 4 deletions.
diff --git a/NEWS b/NEWS
@@ -51,6 +51,8 @@ PHP                                                                        NEWS
     SplFileObject::__constructor). (Girgias)
   . Fixed bug GH-16589 (UAF in SplDoublyLinked->serialize()). (nielsdos)
   . Fixed bug GH-16604 (Memory leaks in SPL constructors). (nielsdos)
+  . Fixed bug GH-16646 (UAF in ArrayObject::unset() and
+    ArrayObject::exchangeArray()). (ilutov)
 
 - SysVShm:
   . Fixed bug GH-16591 (Assertion error in shm_put_var). (nielsdos, cmb)

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
@@ -553,13 +553,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);
@@ -931,8 +933,10 @@ static zend_result spl_array_skip_protected(spl_array_object *intern, HashTable
 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 {
@@ -950,7 +954,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) {
-			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;
@@ -968,9 +972,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);
 		}
 	}
@@ -981,6 +986,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);
 }
 /* }}} */
 

diff --git a/ext/spl/tests/gh16646.phpt b/ext/spl/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) {
+  }
+}
diff --git a/ext/spl/tests/gh16646_2.phpt b/ext/spl/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) {
+  }
+}