Fix keyed iterator value lifetime across reentrant key() calls

NEWS

@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.4.24
 
+- Core:
+  . Fixed use-after-free in keyed foreach over a reentrant iterator.
+    (PuH4ck3rX)
+
 - Calendar:
   . Fixed bug GH-22602 (gregoriantojd() and juliantojd() integer overflow with
     INT_MAX year). (arshidkv12)
@@ -73,6 +77,8 @@ PHP                                                                        NEWS
     PHP 8.3 and PHP 8.5). (jorgsowa)
 
 - SPL:
+  . Fixed use-after-free in iterator_to_array() with a reentrant key()
+    implementation. (PuH4ck3rX)
   . Fix class_parents for classes with leading slash in non-autoload mode.
     (jorgsowa)
   . Ignore leading back-slash in class_parents(), class_implements(), and

Zend/zend_vm_def.h

@@ -7014,7 +7014,9 @@ ZEND_VM_HELPER(zend_fe_fetch_object_helper, ANY, ANY)
 	USE_OPLINE
 	zval *array;
 	zval *value;
+	zval value_copy;
 	uint32_t value_type;
+	bool release_value = false;
 	HashTable *fe_ht;
 	HashPosition pos;
 	Bucket *p;
@@ -7100,11 +7102,15 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):
 		}
 		if (RETURN_VALUE_USED(opline)) {
 			if (funcs->get_current_key) {
+				ZVAL_COPY(&value_copy, value);
+				release_value = true;
 				funcs->get_current_key(iter, EX_VAR(opline->result.var));
 				if (UNEXPECTED(EG(exception) != NULL)) {
+					zval_ptr_dtor(&value_copy);
 					UNDEF_RESULT();
 					HANDLE_EXCEPTION();
 				}
+				value = &value_copy;
 			} else {
 				ZVAL_LONG(EX_VAR(opline->result.var), iter->index);
 			}
@@ -7115,6 +7121,9 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):
 	if (EXPECTED(OP2_TYPE == IS_CV)) {
 		zval *variable_ptr = EX_VAR(opline->op2.var);
 		zend_assign_to_variable(variable_ptr, value, IS_CV, EX_USES_STRICT_TYPES());
+		if (release_value) {
+			zval_ptr_dtor(&value_copy);
+		}
 	} else {
 		zval *res = EX_VAR(opline->op2.var);
 		zend_refcounted *gc = Z_COUNTED_P(value);
@@ -7123,6 +7132,9 @@ ZEND_VM_C_LABEL(fe_fetch_r_exit):
 		if (Z_TYPE_INFO_REFCOUNTED(value_type)) {
 			GC_ADDREF(gc);
 		}
+		if (release_value) {
+			zval_ptr_dtor(&value_copy);
+		}
 	}
 	ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
 }

ext/spl/spl_iterators.c

@@ -3007,13 +3007,16 @@ static int spl_iterator_to_array_apply(zend_object_iterator *iter, void *puser)
 		return ZEND_HASH_APPLY_STOP;
 	}
 	if (iter->funcs->get_current_key) {
-		zval key;
+		zval key, data_copy;
+		ZVAL_COPY(&data_copy, data);
 		iter->funcs->get_current_key(iter, &key);
 		if (EG(exception)) {
+			zval_ptr_dtor(&data_copy);
 			return ZEND_HASH_APPLY_STOP;
 		}
-		array_set_zval_key(Z_ARRVAL_P(return_value), &key, data);
+		array_set_zval_key(Z_ARRVAL_P(return_value), &key, &data_copy);
 		zval_ptr_dtor(&key);
+		zval_ptr_dtor(&data_copy);
 	} else {
 		Z_TRY_ADDREF_P(data);
 		add_next_index_zval(return_value, data);

ext/spl/tests/keyed_iterator_reentrant_current_data.phpt

@@ -0,0 +1,86 @@
+--TEST--
+Keyed iterator consumers retain the current value across reentrant key() calls
+--FILE--
+<?php
+
+class ReentrantKeyIterator implements RecursiveIterator {
+    private int $index = 0;
+
+    public function __construct(
+        private array $values,
+        private bool $leaf = false,
+    ) {}
+
+    public function rewind(): void {
+        $this->index = 0;
+    }
+
+    public function valid(): bool {
+        return $this->index < count($this->values);
+    }
+
+    public function current(): mixed {
+        return $this->values[$this->index];
+    }
+
+    public function key(): mixed {
+        global $iterator, $advanced, $spraySource, $holder;
+
+        $key = $this->index;
+        if ($this->leaf && !$advanced) {
+            $advanced = true;
+            $iterator->next();
+
+            $holder = new IteratorIterator($spraySource);
+            $holder->rewind();
+            $holder->current();
+        }
+
+        return $key;
+    }
+
+    public function next(): void {
+        ++$this->index;
+    }
+
+    public function hasChildren(): bool {
+        return !$this->leaf;
+    }
+
+    public function getChildren(): ?RecursiveIterator {
+        return $this->values[$this->index];
+    }
+}
+
+function createIterator(): RecursiveIteratorIterator {
+    global $iterator, $advanced, $spraySource, $holder;
+
+    $advanced = false;
+    $holder = null;
+    $spraySource = new ReentrantKeyIterator(['replacement'], true);
+    $leaf = new ReentrantKeyIterator(['original'], true);
+    $root = new ReentrantKeyIterator([$leaf]);
+    return $iterator = new RecursiveIteratorIterator($root);
+}
+
+var_dump(iterator_to_array(createIterator(), true));
+
+foreach (createIterator() as $key => $value) {
+    var_dump($key, $value);
+}
+
+$target = new stdClass();
+foreach (createIterator() as $key => $target->value) {
+    var_dump($key, $target->value);
+}
+
+?>
+--EXPECT--
+array(1) {
+  [0]=>
+  string(8) "original"
+}
+int(0)
+string(8) "original"
+int(0)
+string(8) "original"