diff --git a/NEWS b/NEWS index 7324e5c591aa..b584c999e5af 100644 --- a/NEWS +++ b/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 diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index c46b17101546..7bf3e5ae96e3 100644 --- a/Zend/zend_vm_def.h +++ b/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(); } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index f4f35ce2a8a9..cda7b76af978 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -2901,7 +2901,9 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_fe_fetch_obj USE_OPLINE zval *array; zval *value; + zval value_copy; uint32_t value_type; + bool release_value = false; HashTable *fe_ht; HashPosition pos; Bucket *p; @@ -2987,11 +2989,15 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_fe_fetch_obj } 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); } @@ -3002,6 +3008,9 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_fe_fetch_obj if (EXPECTED(opline->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); @@ -3010,6 +3019,9 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_fe_fetch_obj if (Z_TYPE_INFO_REFCOUNTED(value_type)) { GC_ADDREF(gc); } + if (release_value) { + zval_ptr_dtor(&value_copy); + } } ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION(); } diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index 425bd99f37c0..bbf7340b74f9 100644 --- a/ext/spl/spl_iterators.c +++ b/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); diff --git a/ext/spl/tests/keyed_iterator_reentrant_current_data.phpt b/ext/spl/tests/keyed_iterator_reentrant_current_data.phpt new file mode 100644 index 000000000000..ff5454965f16 --- /dev/null +++ b/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-- +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"