Fix GH-16649: Avoid UAF when using array_splice

Author: alexandre-daubois

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)

Zend/tests/gh16649.phpt

@@ -0,0 +1,47 @@
+--TEST--
+GH-16649: Use-after-free during array_splice
+--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"];
+
+array_splice($arr, 1, 2);
+
+var_dump($arr);
+
+?>
+--EXPECT--
+array(9) {
+  ["a"]=>
+  string(1) "1"
+  [0]=>
+  int(1)
+  [1]=>
+  int(3)
+  [2]=>
+  int(4)
+  [3]=>
+  int(5)
+  [4]=>
+  int(6)
+  [5]=>
+  int(7)
+  [6]=>
+  int(8)
+  [7]=>
+  int(9)
+}

ext/standard/array.c

@@ -3252,7 +3252,8 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 
 		/* If hash for removed entries exists, go until offset+length and copy the entries to it */
 		if (removed != NULL) {
-			for ( ; pos - offset < length && idx < in_hash->nNumUsed; idx++, entry++) {
+			for ( ; pos - offset < length && idx < in_hash->nNumUsed; idx++) {
+				entry = in_hash->arPacked + idx;
 				if (Z_TYPE_P(entry) == IS_UNDEF) continue;
 				pos++;
 				Z_TRY_ADDREF_P(entry);
@@ -3262,7 +3263,8 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 		} else { /* otherwise just skip those entries */
 			zend_long pos2 = pos;
 
-			for ( ; pos2 - offset < length && idx < in_hash->nNumUsed; idx++, entry++) {
+			for ( ; pos2 - offset < length && idx < in_hash->nNumUsed; idx++) {
+				entry = in_hash->arPacked + idx;
 				if (Z_TYPE_P(entry) == IS_UNDEF) continue;
 				pos2++;
 				zend_hash_packed_del_val(in_hash, entry);
@@ -3317,7 +3319,8 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 
 		/* If hash for removed entries exists, go until offset+length and copy the entries to it */
 		if (removed != NULL) {
-			for ( ; pos - offset < length && idx < in_hash->nNumUsed; idx++, p++) {
+			for ( ; pos - offset < length && idx < in_hash->nNumUsed; idx++) {
+				p = in_hash->arData + idx;
 				if (Z_TYPE(p->val) == IS_UNDEF) continue;
 				pos++;
 				entry = &p->val;
@@ -3332,7 +3335,8 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 		} else { /* otherwise just skip those entries */
 			zend_long pos2 = pos;
 
-			for ( ; pos2 - offset < length && idx < in_hash->nNumUsed; idx++, p++) {
+			for ( ; pos2 - offset < length && idx < in_hash->nNumUsed; idx++) {
+				p = in_hash->arData + idx;
 				if (Z_TYPE(p->val) == IS_UNDEF) continue;
 				pos2++;
 				zend_hash_del_bucket(in_hash, p);
@@ -3350,7 +3354,8 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H
 		}
 
 		/* Copy the remaining input hash entries to the output hash */
-		for ( ; idx < in_hash->nNumUsed ; idx++, p++) {
+		for ( ; idx < in_hash->nNumUsed ; idx++) {
+			p = in_hash->arData + idx;
 			if (Z_TYPE(p->val) == IS_UNDEF) continue;
 			entry = &p->val;
 			if (p->key == NULL) {