Fix GH-22678: array_multisort() use-after-free on mutating comparator

NEWS

@@ -85,6 +85,8 @@ PHP                                                                        NEWS
     (Weilin Du)
   . Fixed integer overflow in getimagesize() and getimagesizefromstring()
     when parsing an IFF chunk with a size of INT_MAX. (David Carlier, Weilin Du)
+  . Fixed bug GH-22678 (Use-after-free in array_multisort() when the comparator
+    mutates the array being sorted). (iliaal)
 
 - Zip:
   . Fixed bug GH-22649 (ZipArchive::setCommentName() and setCommentIndex()

ext/standard/array.c

@@ -6046,6 +6046,7 @@ PHP_FUNCTION(array_multisort)
 {
 	zval*			args;
 	zval**			arrays;
+	HashTable**		hashes;
 	Bucket**		indirect;
 	uint32_t		idx;
 	HashTable*		hash;
@@ -6167,20 +6168,26 @@ PHP_FUNCTION(array_multisort)
 	for (i = 0; i < array_size; i++) {
 		indirect[i] = indirects + (i * (num_arrays + 1));
 	}
+	hashes = (HashTable **)safe_emalloc(num_arrays, sizeof(HashTable *), 0);
+	for (i = 0; i < num_arrays; i++) {
+		hashes[i] = Z_ARRVAL_P(arrays[i]);
+		GC_ADDREF(hashes[i]);
+		HT_ALLOW_COW_VIOLATION(hashes[i]);
+	}
 	for (i = 0; i < num_arrays; i++) {
 		k = 0;
-		if (HT_IS_PACKED(Z_ARRVAL_P(arrays[i]))) {
-			zval *zv = Z_ARRVAL_P(arrays[i])->arPacked;
-			for (idx = 0; idx < Z_ARRVAL_P(arrays[i])->nNumUsed; idx++, zv++) {
+		if (HT_IS_PACKED(hashes[i])) {
+			zval *zv = hashes[i]->arPacked;
+			for (idx = 0; idx < hashes[i]->nNumUsed; idx++, zv++) {
 				if (Z_TYPE_P(zv) == IS_UNDEF) continue;
 				ZVAL_COPY_VALUE(&indirect[k][i].val, zv);
 				indirect[k][i].h = idx;
 				indirect[k][i].key = NULL;
 				k++;
 			}
 		} else {
-			Bucket *p = Z_ARRVAL_P(arrays[i])->arData;
-			for (idx = 0; idx < Z_ARRVAL_P(arrays[i])->nNumUsed; idx++, p++) {
+			Bucket *p = hashes[i]->arData;
+			for (idx = 0; idx < hashes[i]->nNumUsed; idx++, p++) {
 				if (Z_TYPE(p->val) == IS_UNDEF) continue;
 				indirect[k][i] = *p;
 				k++;
@@ -6200,7 +6207,7 @@ PHP_FUNCTION(array_multisort)
 
 	/* Restructure the arrays based on sorted indirect - this is mostly taken from zend_hash_sort() function. */
 	for (i = 0; i < num_arrays; i++) {
-		hash = Z_ARRVAL_P(arrays[i]);
+		hash = hashes[i];
 		hash->nNumUsed = array_size;
 		hash->nNextFreeElement = array_size;
 		hash->nInternalPointer = 0;
@@ -6229,6 +6236,14 @@ PHP_FUNCTION(array_multisort)
 	RETVAL_TRUE;
 
 clean_up:
+	for (i = 0; i < num_arrays; i++) {
+		if (UNEXPECTED(GC_DELREF(hashes[i]) == 0)) {
+			zend_array_destroy(hashes[i]);
+		} else {
+			gc_check_possible_root((zend_refcounted *)hashes[i]);
+		}
+	}
+	efree(hashes);
 	efree(indirects);
 	efree(indirect);
 	efree(func);

ext/standard/tests/array/gh22678.phpt

@@ -0,0 +1,48 @@
+--TEST--
+GH-22678 (Use-after-free in array_multisort() when the comparator mutates the array)
+--FILE--
+<?php
+class Evil {
+    public function __toString(): string {
+        // Runs from the SORT_STRING comparator; drop the array being sorted.
+        foreach (array_keys($GLOBALS['a']) as $k) {
+            unset($GLOBALS['a'][$k]);
+        }
+        $GLOBALS['a'] = [];
+        return "x";
+    }
+}
+
+$a = [];
+for ($i = 0; $i < 10; $i++) {
+    $a[] = new Evil();
+}
+$GLOBALS['a'] = &$a;
+
+echo "freed mid-sort: ";
+var_dump(array_multisort($a, SORT_STRING));
+unset($GLOBALS['a']);
+
+// Non-packed integer keys reach the mixed-to-packed restructure while the sort
+// holds its reference on the array.
+$b = [5 => 'c', 3 => 'a', 1 => 'b'];
+array_multisort($b, SORT_STRING);
+echo "repacked: ", implode(',', $b), "\n";
+
+class Boom {
+    public function __toString(): string {
+        throw new Exception("boom");
+    }
+}
+
+$c = [new Boom(), new Boom()];
+try {
+    array_multisort($c, SORT_STRING);
+} catch (Exception $e) {
+    echo $e::class, ": ", $e->getMessage(), "\n";
+}
+?>
+--EXPECT--
+freed mid-sort: bool(true)
+repacked: a,b,c
+Exception: boom