From 139295368009b0ce6972b94e691868ef50f06942 Mon Sep 17 00:00:00 2001
From: Alexandre Daubois <alex.daubois@gmail.com>
Date: Thu, 7 Aug 2025 09:41:50 +0200
Subject: [PATCH] Fix GH-16649: Avoid UAF when using array_splice

---
 NEWS                                          |  1 +
 ext/standard/array.c                          | 12 +++++++
 .../array_splice_normal_destructor.phpt       | 20 +++++++++++
 .../array_splice_uaf_add_elements.phpt        | 22 +++++++++++++
 .../array_splice_uaf_array_deallocated.phpt   | 22 +++++++++++++
 ...array_splice_uaf_complex_modification.phpt | 25 ++++++++++++++
 ...array_splice_uaf_multiple_destructors.phpt | 33 +++++++++++++++++++
 .../array_splice_uaf_original_case.phpt       | 29 ++++++++++++++++
 .../array_splice_uaf_packed_to_hash.phpt      | 23 +++++++++++++
 .../array_splice_with_replacement.phpt        | 23 +++++++++++++
 10 files changed, 210 insertions(+)
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt
 create mode 100644 ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt

diff --git a/NEWS b/NEWS
index ad06a4eef1b7b..b13c914d650f9 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,7 @@ PHP                                                                        NEWS
   . Fix theoretical issues with hrtime() not being available. (nielsdos)
   . Fixed bug GH-19300 (Nested array_multisort invocation with error breaks).
     (nielsdos)
+  . Fixed bug GH-16649 (UAF during array_splice). (alexandre-daubois)
 
 - Windows:
   . Free opened_path when opened_path_len >= MAXPATHLEN. (dixyes)
diff --git a/ext/standard/array.c b/ext/standard/array.c
index a2a0459ec3b29..45e36ba09ce70 100644
--- a/ext/standard/array.c
+++ b/ext/standard/array.c
@@ -3214,6 +3214,9 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 	zval		*entry;				/* Hash entry */
 	uint32_t    iter_pos = zend_hash_iterators_lower_pos(in_hash, 0);
 
+	GC_ADDREF(in_hash);
+	HT_ALLOW_COW_VIOLATION(in_hash); /* Will be reset when setting the flags for in_hash */
+
 	/* Get number of entries in the input hash */
 	num_in = zend_hash_num_elements(in_hash);
 
@@ -3372,6 +3375,15 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 	HT_SET_ITERATORS_COUNT(&out_hash, HT_ITERATORS_COUNT(in_hash));
 	HT_SET_ITERATORS_COUNT(in_hash, 0);
 	in_hash->pDestructor = NULL;
+	
+	if (UNEXPECTED(GC_DELREF(in_hash) == 0)) {
+		/* Array was completely deallocated during the operation */
+		zend_array_destroy(in_hash);
+		zend_hash_destroy(&out_hash);
+		zend_throw_error(NULL, "Array was modified during array_splice operation");
+		return;
+	}
+
 	zend_hash_destroy(in_hash);
 
 	HT_FLAGS(in_hash)          = HT_FLAGS(&out_hash);
diff --git a/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt b/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt
new file mode 100644
index 0000000000000..59c0b316f54d5
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt
@@ -0,0 +1,20 @@
+--TEST--
+GH-16649: array_splice with normal destructor should work fine
+--FILE--
+<?php
+class C {
+    function __destruct() {
+        echo "Destructor called\n";
+    }
+}
+
+$arr = ["1", new C, "2"];
+array_splice($arr, 1, 2);
+var_dump($arr);
+?>
+--EXPECT--
+Destructor called
+array(1) {
+  [0]=>
+  string(1) "1"
+}
diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt
new file mode 100644
index 0000000000000..f5e538122f415
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt
@@ -0,0 +1,22 @@
+--TEST--
+GH-16649: array_splice UAF when destructor adds elements to array
+--FILE--
+<?php
+class C {
+    function __destruct() {
+        global $arr;
+        $arr[] = 0;
+    }
+}
+
+$arr = ["1", new C, "2"];
+
+try {
+    array_splice($arr, 1, 2);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Exception caught: Array was modified during array_splice operation
diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt
new file mode 100644
index 0000000000000..1874ddad8934d
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt
@@ -0,0 +1,22 @@
+--TEST--
+GH-16649: array_splice UAF when array is released entirely
+--FILE--
+<?php
+class C {
+    function __destruct() {
+        global $arr;
+        $arr = null;
+    }
+}
+
+$arr = ["1", new C, "2"];
+
+try {
+    array_splice($arr, 1, 2);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Exception caught: Array was modified during array_splice operation
diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt
new file mode 100644
index 0000000000000..8ad4133300ec5
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt
@@ -0,0 +1,25 @@
+--TEST--
+GH-16649: array_splice UAF with complex array modification
+--FILE--
+<?php
+class ComplexModifier {
+    function __destruct() {
+        global $arr;
+        // complex modification that causes cow
+        unset($arr[0]);
+        $arr["new_key"] = "new_value";
+        $arr[100] = "another_value";
+    }
+}
+
+$arr = ["first", new ComplexModifier, "last"];
+
+try {
+    array_splice($arr, 1, 1, ["replacement"]);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Exception caught: Array was modified during array_splice operation
diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt
new file mode 100644
index 0000000000000..88c6e4286c4d6
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt
@@ -0,0 +1,33 @@
+--TEST--
+GH-16649: array_splice UAF with multiple destructors
+--FILE--
+<?php
+class MultiDestructor {
+    public $id;
+    
+    function __construct($id) { 
+        $this->id = $id; 
+    }
+    
+    function __destruct() {
+        global $arr;
+        echo "Destructor {$this->id} called\n";
+        if ($this->id == 2) {
+            $arr = null;
+        }
+    }
+}
+
+$arr = ["start", new MultiDestructor(1), new MultiDestructor(2), "end"];
+
+try {
+    array_splice($arr, 1, 2);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Destructor 1 called
+Destructor 2 called
+Exception caught: Array was modified during array_splice operation
diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt
new file mode 100644
index 0000000000000..4a82d5893157d
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt
@@ -0,0 +1,29 @@
+--TEST--
+GH-16649: array_splice UAF with destructor modifying array (original case)
+--FILE--
+<?php
+function resize_arr() {
+    global $arr;
+    for ($i = 0; $i < 10; $i++) {
+        $arr[$i] = $i;
+    }
+}
+
+class C {
+    function __destruct() {
+        resize_arr();
+        return "3";
+    }
+}
+
+$arr = ["a" => "1", "3" => new C, "2" => "2"];
+
+try {
+    array_splice($arr, 1, 2);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Exception caught: Array was modified during array_splice operation
diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt
new file mode 100644
index 0000000000000..bd8f511b6253e
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt
@@ -0,0 +1,23 @@
+--TEST--
+GH-16649: array_splice UAF when array is converted from packed to hash
+--FILE--
+<?php
+class C {
+    function __destruct() {
+        global $arr;
+        // array is converted from packed to hash
+        $arr["str"] = 0;
+    }
+}
+
+$arr = ["1", new C, "2"];
+
+try {
+    array_splice($arr, 1, 2);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Exception caught: Array was modified during array_splice operation
diff --git a/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt b/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt
new file mode 100644
index 0000000000000..8754a913a24e4
--- /dev/null
+++ b/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt
@@ -0,0 +1,23 @@
+--TEST--
+GH-16649: array_splice with replacement array when destructor modifies array
+--FILE--
+<?php
+class C {
+    function __destruct() {
+        global $arr;
+        $arr["modified"] = "by_destructor";
+    }
+}
+
+$arr = ["a", new C, "b"];
+$replacement = ["replacement1", "replacement2"];
+
+try {
+    array_splice($arr, 1, 1, $replacement);
+    echo "ERROR: Should have thrown exception\n";
+} catch (Error $e) {
+    echo "Exception caught: " . $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Exception caught: Array was modified during array_splice operation
