From b074c5cc551f3f985cefe0267e50968191403114 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Fri, 28 Nov 2025 18:45:32 +0100 Subject: [PATCH 1/6] reflection: Remove dead code Since the executor needs to be active at this point, the only way you could get an UNDEF return value is by having an exception. Therefore, `!EG(exception)` is always false. The check doesn't make sense, remove it. --- ext/reflection/php_reflection.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index a86ce16feb407..3327fdc7f6f6d 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -2141,12 +2141,6 @@ ZEND_METHOD(ReflectionFunction, invoke) zend_call_known_fcc(&fcc, &retval, num_args, params, named_params); - if (Z_TYPE(retval) == IS_UNDEF && !EG(exception)) { - zend_throw_exception_ex(reflection_exception_ptr, 0, - "Invocation of function %s() failed", ZSTR_VAL(fptr->common.function_name)); - RETURN_THROWS(); - } - if (Z_ISREF(retval)) { zend_unwrap_reference(&retval); } @@ -2180,12 +2174,6 @@ ZEND_METHOD(ReflectionFunction, invokeArgs) zend_call_known_fcc(&fcc, &retval, /* num_params */ 0, /* params */ NULL, params); - if (Z_TYPE(retval) == IS_UNDEF && !EG(exception)) { - zend_throw_exception_ex(reflection_exception_ptr, 0, - "Invocation of function %s() failed", ZSTR_VAL(fptr->common.function_name)); - RETURN_THROWS(); - } - if (Z_ISREF(retval)) { zend_unwrap_reference(&retval); } @@ -3496,12 +3484,6 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) callback = _copy_function(mptr); zend_call_known_function(callback, (object ? Z_OBJ_P(object) : NULL), intern->ce, &retval, argc, params, named_params); - if (Z_TYPE(retval) == IS_UNDEF && !EG(exception)) { - zend_throw_exception_ex(reflection_exception_ptr, 0, - "Invocation of method %s::%s() failed", ZSTR_VAL(mptr->common.scope->name), ZSTR_VAL(mptr->common.function_name)); - RETURN_THROWS(); - } - if (Z_ISREF(retval)) { zend_unwrap_reference(&retval); } From b64cd429ca4f6cadf9220c0a0cba4afbfe101687 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Fri, 28 Nov 2025 18:46:52 +0100 Subject: [PATCH 2/6] reflection: Use RETURN_COPY_VALUE() --- ext/reflection/php_reflection.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 3327fdc7f6f6d..05454b24c4e68 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -2144,7 +2144,7 @@ ZEND_METHOD(ReflectionFunction, invoke) if (Z_ISREF(retval)) { zend_unwrap_reference(&retval); } - ZVAL_COPY_VALUE(return_value, &retval); + RETURN_COPY_VALUE(&retval); } /* }}} */ @@ -2177,7 +2177,7 @@ ZEND_METHOD(ReflectionFunction, invokeArgs) if (Z_ISREF(retval)) { zend_unwrap_reference(&retval); } - ZVAL_COPY_VALUE(return_value, &retval); + RETURN_COPY_VALUE(&retval); } /* }}} */ @@ -3487,7 +3487,7 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) if (Z_ISREF(retval)) { zend_unwrap_reference(&retval); } - ZVAL_COPY_VALUE(return_value, &retval); + RETURN_COPY_VALUE(&retval); } /* }}} */ From e025f2a14846ed5f9a83df7ed811aaf3043a4dd4 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Sun, 30 Nov 2025 01:39:04 -0800 Subject: [PATCH 3/6] reflection: Test ReflectionFunction::__toString() with bound variables (#20608) --- ...ionFunction__toString_bound_variables.phpt | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 ext/reflection/tests/ReflectionFunction__toString_bound_variables.phpt diff --git a/ext/reflection/tests/ReflectionFunction__toString_bound_variables.phpt b/ext/reflection/tests/ReflectionFunction__toString_bound_variables.phpt new file mode 100644 index 0000000000000..142e661361496 --- /dev/null +++ b/ext/reflection/tests/ReflectionFunction__toString_bound_variables.phpt @@ -0,0 +1,32 @@ +--TEST-- +ReflectionFunction::__toString() with bound variables +--FILE-- + 0; + +$rf = new ReflectionFunction($closure_without_bounds); +echo (string) $rf; + +$global = ""; +$closure_with_bounds = function() use($global) { + static $counter = 0; + return $counter++; +}; + +$rf = new ReflectionFunction($closure_with_bounds); +echo (string) $rf; + +?> +--EXPECTF-- +Closure [ function {closure:%s:%d} ] { + @@ %sReflectionFunction__toString_bound_variables.php 3 - 3 +} +Closure [ function {closure:%s:%d} ] { + @@ %sReflectionFunction__toString_bound_variables.php 9 - 12 + + - Bound Variables [2] { + Variable #0 [ $global ] + Variable #1 [ $counter ] + } +} From 13bf672cdb1bcd9684225f6f0473c071434bdc8c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Fri, 28 Nov 2025 19:03:22 +0100 Subject: [PATCH 4/6] reflection: Use zend_hash_str_find_ptr_lc() where possible --- ext/reflection/php_reflection.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 05454b24c4e68..4cf3edd47375e 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -1432,13 +1432,7 @@ static void reflection_extension_factory_ex(zval *object, zend_module_entry *mod static void reflection_extension_factory(zval *object, const char *name_str) { size_t name_len = strlen(name_str); - zend_string *lcname; - struct _zend_module_entry *module; - - lcname = zend_string_alloc(name_len, 0); - zend_str_tolower_copy(ZSTR_VAL(lcname), name_str, name_len); - module = zend_hash_find_ptr(&module_registry, lcname); - zend_string_efree(lcname); + struct _zend_module_entry *module = zend_hash_str_find_ptr_lc(&module_registry, name_str, name_len); if (!module) { return; } @@ -6643,12 +6637,10 @@ ZEND_METHOD(ReflectionProperty, isFinal) ZEND_METHOD(ReflectionExtension, __construct) { zval *object; - char *lcname; reflection_object *intern; zend_module_entry *module; char *name_str; size_t name_len; - ALLOCA_FLAG(use_heap) if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &name_str, &name_len) == FAILURE) { RETURN_THROWS(); @@ -6656,15 +6648,11 @@ ZEND_METHOD(ReflectionExtension, __construct) object = ZEND_THIS; intern = Z_REFLECTION_P(object); - lcname = do_alloca(name_len + 1, use_heap); - zend_str_tolower_copy(lcname, name_str, name_len); - if ((module = zend_hash_str_find_ptr(&module_registry, lcname, name_len)) == NULL) { - free_alloca(lcname, use_heap); + if ((module = zend_hash_str_find_ptr_lc(&module_registry, name_str, name_len)) == NULL) { zend_throw_exception_ex(reflection_exception_ptr, 0, "Extension \"%s\" does not exist", name_str); RETURN_THROWS(); } - free_alloca(lcname, use_heap); zval *prop_name = reflection_prop_name(object); zval_ptr_dtor(prop_name); ZVAL_STRING(prop_name, module->name); From 157864af49eab6561e02412e7838d9a195968efe Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Fri, 28 Nov 2025 19:47:44 +0100 Subject: [PATCH 5/6] reflection: Use zend_hash_find_ptr_lc() where possible --- ext/reflection/php_reflection.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 4cf3edd47375e..d6e55c982b425 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -6639,18 +6639,17 @@ ZEND_METHOD(ReflectionExtension, __construct) zval *object; reflection_object *intern; zend_module_entry *module; - char *name_str; - size_t name_len; + zend_string *name_str; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &name_str, &name_len) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "S", &name_str) == FAILURE) { RETURN_THROWS(); } object = ZEND_THIS; intern = Z_REFLECTION_P(object); - if ((module = zend_hash_str_find_ptr_lc(&module_registry, name_str, name_len)) == NULL) { + if ((module = zend_hash_find_ptr_lc(&module_registry, name_str)) == NULL) { zend_throw_exception_ex(reflection_exception_ptr, 0, - "Extension \"%s\" does not exist", name_str); + "Extension \"%s\" does not exist", ZSTR_VAL(name_str)); RETURN_THROWS(); } zval *prop_name = reflection_prop_name(object); From 366ed4c7508af61dd4d0397d758abfc77ffe2b8a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Sat, 29 Nov 2025 12:07:15 +0100 Subject: [PATCH 6/6] Fix GH-20614: SplFixedArray incorrectly handles references in deserialization All other code caters to dereferencing array elements, except the unserialize handler. This causes references to be present in the fixed array even though this seems not intentional as reference assign is otherwise impossible. On 8.5+ this causes an assertion failure. On 8.3+ this causes references to be present where they shouldn't be. Closes GH-20616. --- NEWS | 4 ++++ ext/spl/spl_fixedarray.c | 4 ++-- ext/spl/tests/gh20614.phpt | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 ext/spl/tests/gh20614.phpt diff --git a/NEWS b/NEWS index 3ec3f8c096d03..5070f818aca29 100644 --- a/NEWS +++ b/NEWS @@ -61,6 +61,10 @@ PHP NEWS . Fixed ZPP type violation in phpdbg_get_executable() and phpdbg_end_oplog(). (Girgias) +- SPL: + . Fixed bug GH-20614 (SplFixedArray incorrectly handles references + in deserialization). (ndossche) + - Standard: . Fix memory leak in array_diff() with custom type checks. (ndossche) . Fixed bug GH-20583 (Stack overflow in http_build_query diff --git a/ext/spl/spl_fixedarray.c b/ext/spl/spl_fixedarray.c index 49eb8841de1b2..53dba1b727cf7 100644 --- a/ext/spl/spl_fixedarray.c +++ b/ext/spl/spl_fixedarray.c @@ -652,7 +652,7 @@ PHP_METHOD(SplFixedArray, __unserialize) intern->array.size = 0; ZEND_HASH_FOREACH_STR_KEY_VAL(data, key, elem) { if (key == NULL) { - ZVAL_COPY(&intern->array.elements[intern->array.size], elem); + ZVAL_COPY_DEREF(&intern->array.elements[intern->array.size], elem); intern->array.size++; } else { Z_TRY_ADDREF_P(elem); @@ -833,7 +833,7 @@ PHP_METHOD(SplFixedArray, offsetGet) value = spl_fixedarray_object_read_dimension_helper(intern, zindex); if (value) { - RETURN_COPY_DEREF(value); + RETURN_COPY(value); } else { RETURN_NULL(); } diff --git a/ext/spl/tests/gh20614.phpt b/ext/spl/tests/gh20614.phpt new file mode 100644 index 0000000000000..c13630d76469b --- /dev/null +++ b/ext/spl/tests/gh20614.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-20614 (SplFixedArray incorrectly handles references in deserialization) +--FILE-- +__unserialize($array); +var_dump($fa); +unset($fa[0]); +var_dump($fa); + +?> +--EXPECT-- +object(SplFixedArray)#1 (1) { + [0]=> + int(1) +} +object(SplFixedArray)#1 (1) { + [0]=> + NULL +}