From 277d3016c06f55e181bc80eeff7745cf0048b631 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 6 Mar 2026 17:21:00 -0500 Subject: [PATCH 1/4] Fix GH-21362: ReflectionMethod::invoke/invokeArgs() rejects different Closure instances ReflectionMethod::invokeArgs() (and invoke()) for Closure::__invoke() incorrectly accepted any Closure object, not just the one the ReflectionMethod was created from. This happened because all Closures share a single zend_ce_closure class entry, so the instanceof_function() check always passed. Fix: store the original Closure object in intern->obj during ReflectionMethod construction, then compare object identity in reflection_method_invoke() to reject different Closure instances. Closes GH-21362 --- ext/reflection/php_reflection.c | 21 +++++++++++- ext/reflection/tests/gh21362.phpt | 53 +++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 ext/reflection/tests/gh21362.phpt diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 587bca11522b..3d800041595b 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3306,7 +3306,9 @@ static void instantiate_reflection_method(INTERNAL_FUNCTION_PARAMETERS, bool is_ && memcmp(lcname, ZEND_INVOKE_FUNC_NAME, sizeof(ZEND_INVOKE_FUNC_NAME)-1) == 0 && (mptr = zend_get_closure_invoke_method(orig_obj)) != NULL) { - /* do nothing, mptr already set */ + /* Store the original closure object so we can validate it in invoke/invokeArgs. + * Each closure has a unique __invoke signature, so we must reject different closures. */ + ZVAL_OBJ_COPY(&intern->obj, orig_obj); } else if ((mptr = zend_hash_str_find_ptr(&ce->function_table, lcname, method_name_len)) == NULL) { efree(lcname); zend_throw_exception_ex(reflection_exception_ptr, 0, @@ -3441,6 +3443,23 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) _DO_THROW("Given object is not an instance of the class this method was declared in"); RETURN_THROWS(); } + + /* For Closure::__invoke(), closures from different source locations have + * different signatures, so we must reject those. However, closures created + * from the same source (e.g. in a loop) share the same op_array and should + * be allowed. Compare the underlying function pointer via op_array. */ + if (obj_ce == zend_ce_closure && !Z_ISUNDEF(intern->obj) + && Z_OBJ_P(object) != Z_OBJ(intern->obj)) { + const zend_function *orig_func = zend_get_closure_method_def(Z_OBJ(intern->obj)); + const zend_function *given_func = zend_get_closure_method_def(Z_OBJ_P(object)); + if (orig_func->op_array.opcodes != given_func->op_array.opcodes) { + if (!variadic) { + efree(params); + } + _DO_THROW("Given object is not an instance of the class this method was declared in"); + RETURN_THROWS(); + } + } } /* Copy the zend_function when calling via handler (e.g. Closure::__invoke()) */ callback = _copy_function(mptr); diff --git a/ext/reflection/tests/gh21362.phpt b/ext/reflection/tests/gh21362.phpt new file mode 100644 index 000000000000..d7aac3b6f548 --- /dev/null +++ b/ext/reflection/tests/gh21362.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-21362 (ReflectionMethod::invokeArgs() for Closure::__invoke() accepts objects from different Closures) +--FILE-- +invokeArgs($c1, ['foo' => 'FOO', 'bar' => 'BAR']); + +// invokeArgs with a different Closure should throw +try { + $m->invokeArgs($c2, ['foo' => 'FOO', 'bar' => 'BAR']); + echo "No exception thrown\n"; +} catch (ReflectionException $e) { + echo $e->getMessage() . "\n"; +} + +// invoke with a different Closure should also throw +try { + $m->invoke($c2, 'FOO', 'BAR'); + echo "No exception thrown\n"; +} catch (ReflectionException $e) { + echo $e->getMessage() . "\n"; +} + +// Closures from the same source (e.g. loop) share the same op_array +// and should be allowed to invoke each other's ReflectionMethod +$closures = []; +for ($i = 0; $i < 3; $i++) { + $closures[] = function () use ($i) { return $i; }; +} + +$m2 = new ReflectionMethod($closures[0], '__invoke'); +foreach ($closures as $closure) { + var_dump($m2->invoke($closure)); +} +?> +--EXPECT-- +c1: foo=FOO, bar=BAR +Given object is not an instance of the class this method was declared in +Given object is not an instance of the class this method was declared in +int(0) +int(1) +int(2) From 61d4a391f1b9725ea884702d50268317a84d5057 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 7 Mar 2026 12:49:45 -0500 Subject: [PATCH 2/4] Improve error message for incompatible Closure in ReflectionMethod::invoke() Use a Closure-specific message instead of the generic "not an instance of the class" phrasing, since all Closures share the same class entry. --- ext/reflection/php_reflection.c | 2 +- ext/reflection/tests/gh21362.phpt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 3d800041595b..7f2931c48060 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3456,7 +3456,7 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) if (!variadic) { efree(params); } - _DO_THROW("Given object is not an instance of the class this method was declared in"); + _DO_THROW("Given Closure is not compatible with the reflected Closure"); RETURN_THROWS(); } } diff --git a/ext/reflection/tests/gh21362.phpt b/ext/reflection/tests/gh21362.phpt index d7aac3b6f548..d8156bb0d72d 100644 --- a/ext/reflection/tests/gh21362.phpt +++ b/ext/reflection/tests/gh21362.phpt @@ -46,8 +46,8 @@ foreach ($closures as $closure) { ?> --EXPECT-- c1: foo=FOO, bar=BAR -Given object is not an instance of the class this method was declared in -Given object is not an instance of the class this method was declared in +Given Closure is not compatible with the reflected Closure +Given Closure is not compatible with the reflected Closure int(0) int(1) int(2) From 428198b26ef1ac329aaf624d2f93d3f43362788f Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 7 Mar 2026 17:41:32 -0500 Subject: [PATCH 3/4] Update error message wording per review feedback Change "not compatible with" to "not the same as" since closures with different source locations aren't about compatibility. --- ext/reflection/php_reflection.c | 2 +- ext/reflection/tests/gh21362.phpt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 7f2931c48060..2692e1928068 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3456,7 +3456,7 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) if (!variadic) { efree(params); } - _DO_THROW("Given Closure is not compatible with the reflected Closure"); + _DO_THROW("Given Closure is not the same as the reflected Closure"); RETURN_THROWS(); } } diff --git a/ext/reflection/tests/gh21362.phpt b/ext/reflection/tests/gh21362.phpt index d8156bb0d72d..ce7840b494ed 100644 --- a/ext/reflection/tests/gh21362.phpt +++ b/ext/reflection/tests/gh21362.phpt @@ -46,8 +46,8 @@ foreach ($closures as $closure) { ?> --EXPECT-- c1: foo=FOO, bar=BAR -Given Closure is not compatible with the reflected Closure -Given Closure is not compatible with the reflected Closure +Given Closure is not the same as the reflected Closure +Given Closure is not the same as the reflected Closure int(0) int(1) int(2) From 345eafa8ec00f85e970627cf1abfcb2dbd84dfef Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 7 Mar 2026 19:30:55 -0500 Subject: [PATCH 4/4] Add NEWS entry for GH-21362 Reflection Closure identity fix --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 4e1196fb0946..482dfcb09134 100644 --- a/NEWS +++ b/NEWS @@ -90,6 +90,8 @@ PHP NEWS . Added ReflectionConstant::inNamespace(). (Khaled Alam) . Added ReflectionProperty::isReadable() and ReflectionProperty::isWritable(). (ilutov) + . Fixed bug GH-21362 (ReflectionMethod::invoke/invokeArgs() did not verify + Closure instance identity for Closure::__invoke()). (Ilia Alshanetsky) - Session: . Fixed bug 71162 (updateTimestamp never called when session data is empty).