From 3d1bc50f1b5617c64efd5e7cd6d4a24009fb105e Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 8 Mar 2026 18:08:04 -0400 Subject: [PATCH 1/3] Fix ReflectionMethod::invoke() crash with internal closures The closure identity check added in GH-21366 accessed op_array.opcodes unconditionally, but internal closures (e.g. var_dump(...)) use internal_function, not op_array. This caused undefined behavior when comparing closures created via first-class callable syntax on internal functions. Check the function type first: compare op_array.opcodes for user closures, compare the function pointer directly for internal closures. --- ext/reflection/php_reflection.c | 11 +++++++++-- ext/reflection/tests/gh21362.phpt | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 2692e1928068d..5b8bf9bdb4316 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3447,12 +3447,19 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) /* 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. */ + * be allowed. For user closures compare op_array.opcodes, for internal + * closures (e.g. var_dump(...)) compare the handler pointer. */ 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) { + bool same_closure; + if (orig_func->type == ZEND_USER_FUNCTION && given_func->type == ZEND_USER_FUNCTION) { + same_closure = orig_func->op_array.opcodes == given_func->op_array.opcodes; + } else { + same_closure = orig_func == given_func; + } + if (!same_closure) { if (!variadic) { efree(params); } diff --git a/ext/reflection/tests/gh21362.phpt b/ext/reflection/tests/gh21362.phpt index ce7840b494ed8..c61e8b69af741 100644 --- a/ext/reflection/tests/gh21362.phpt +++ b/ext/reflection/tests/gh21362.phpt @@ -43,6 +43,25 @@ $m2 = new ReflectionMethod($closures[0], '__invoke'); foreach ($closures as $closure) { var_dump($m2->invoke($closure)); } + +// Internal closures (first-class callable syntax) should also be validated +$vd = var_dump(...); +$pr = print_r(...); + +$m3 = new ReflectionMethod($vd, '__invoke'); +$m3->invoke($vd, 'internal closure OK'); + +// Same internal closure, different instance - should work +$vd2 = var_dump(...); +$m3->invoke($vd2, 'same internal closure OK'); + +// Different internal closure should throw +try { + $m3->invoke($pr, 'should not print'); + echo "No exception thrown\n"; +} catch (ReflectionException $e) { + echo $e->getMessage() . "\n"; +} ?> --EXPECT-- c1: foo=FOO, bar=BAR @@ -51,3 +70,6 @@ Given Closure is not the same as the reflected Closure int(0) int(1) int(2) +string(19) "internal closure OK" +string(24) "same internal closure OK" +Given Closure is not the same as the reflected Closure From a794fc93e33dbc28d95d60ba033482c86f5b1250 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 9 Mar 2026 13:32:27 -0400 Subject: [PATCH 2/3] Fix internal closure comparison and expand test coverage The previous comparison (orig_func == given_func) could never match for internal closures since zend_get_closure_method_def() returns a pointer to each closure's embedded copy. Compare function_name and scope instead. Also handle the mixed user/internal type case explicitly. Add tests for: userland first-class callables, cloned internal closures, and cross-type (user vs internal) closure rejection. --- ext/reflection/php_reflection.c | 8 +++-- ext/reflection/tests/gh21362.phpt | 54 +++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 5b8bf9bdb4316..e442f90667e4f 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3448,7 +3448,8 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) * 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. For user closures compare op_array.opcodes, for internal - * closures (e.g. var_dump(...)) compare the handler pointer. */ + * closures (e.g. var_dump(...)) compare function_name and scope since + * zend_get_closure_method_def() returns a per-object embedded copy. */ 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)); @@ -3456,8 +3457,11 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) bool same_closure; if (orig_func->type == ZEND_USER_FUNCTION && given_func->type == ZEND_USER_FUNCTION) { same_closure = orig_func->op_array.opcodes == given_func->op_array.opcodes; + } else if (orig_func->type == ZEND_INTERNAL_FUNCTION && given_func->type == ZEND_INTERNAL_FUNCTION) { + same_closure = orig_func->common.function_name == given_func->common.function_name + && orig_func->common.scope == given_func->common.scope; } else { - same_closure = orig_func == given_func; + same_closure = false; } if (!same_closure) { if (!variadic) { diff --git a/ext/reflection/tests/gh21362.phpt b/ext/reflection/tests/gh21362.phpt index c61e8b69af741..1f5619ce779d0 100644 --- a/ext/reflection/tests/gh21362.phpt +++ b/ext/reflection/tests/gh21362.phpt @@ -44,20 +44,55 @@ foreach ($closures as $closure) { var_dump($m2->invoke($closure)); } +// First-class callable of a userland function +function my_func($x) { return "my_func: $x"; } +function other_func($x) { return "other_func: $x"; } + +$mf = my_func(...); +$mf2 = my_func(...); +$of = other_func(...); + +$m3 = new ReflectionMethod($mf, '__invoke'); +var_dump($m3->invoke($mf, 'test')); +var_dump($m3->invoke($mf2, 'test')); + +try { + $m3->invoke($of, 'test'); + echo "No exception thrown\n"; +} catch (ReflectionException $e) { + echo $e->getMessage() . "\n"; +} + // Internal closures (first-class callable syntax) should also be validated $vd = var_dump(...); $pr = print_r(...); -$m3 = new ReflectionMethod($vd, '__invoke'); -$m3->invoke($vd, 'internal closure OK'); +$m4 = new ReflectionMethod($vd, '__invoke'); +$m4->invoke($vd, 'internal closure OK'); -// Same internal closure, different instance - should work -$vd2 = var_dump(...); -$m3->invoke($vd2, 'same internal closure OK'); +// Cloned internal closure is a different object but same function - should work +$vd2 = clone $vd; +$m4->invoke($vd2, 'cloned internal closure OK'); // Different internal closure should throw try { - $m3->invoke($pr, 'should not print'); + $m4->invoke($pr, 'should not print'); + echo "No exception thrown\n"; +} catch (ReflectionException $e) { + echo $e->getMessage() . "\n"; +} + +// Cross-type: userland Closure to internal Closure's invoke should throw +try { + $m4->invoke($c1, 'should not print'); + echo "No exception thrown\n"; +} catch (ReflectionException $e) { + echo $e->getMessage() . "\n"; +} + +// Cross-type: internal Closure to userland Closure's invoke should throw +try { + $m->invoke($vd, 'should not print'); echo "No exception thrown\n"; } catch (ReflectionException $e) { echo $e->getMessage() . "\n"; @@ -70,6 +105,11 @@ Given Closure is not the same as the reflected Closure int(0) int(1) int(2) +string(13) "my_func: test" +string(13) "my_func: test" +Given Closure is not the same as the reflected Closure string(19) "internal closure OK" -string(24) "same internal closure OK" +string(26) "cloned internal closure OK" +Given Closure is not the same as the reflected Closure +Given Closure is not the same as the reflected Closure Given Closure is not the same as the reflected Closure From 5b037b3ce3916ef665e0025b02b4d78367129f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 9 Mar 2026 20:48:20 +0100 Subject: [PATCH 3/3] php_reflection: Simplify the Closure::__invoke() check --- ext/reflection/php_reflection.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index e442f90667e4f..26d4341c1acdf 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -3445,24 +3445,26 @@ static void reflection_method_invoke(INTERNAL_FUNCTION_PARAMETERS, int variadic) } /* 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. For user closures compare op_array.opcodes, for internal - * closures (e.g. var_dump(...)) compare function_name and scope since - * zend_get_closure_method_def() returns a per-object embedded copy. */ + * different signatures, so we must reject those. */ 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)); - bool same_closure; - if (orig_func->type == ZEND_USER_FUNCTION && given_func->type == ZEND_USER_FUNCTION) { - same_closure = orig_func->op_array.opcodes == given_func->op_array.opcodes; - } else if (orig_func->type == ZEND_INTERNAL_FUNCTION && given_func->type == ZEND_INTERNAL_FUNCTION) { - same_closure = orig_func->common.function_name == given_func->common.function_name - && orig_func->common.scope == given_func->common.scope; - } else { - same_closure = false; + + bool same_closure = false; + /* Check if they are either both fake closures or they both are not. */ + if ((orig_func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE) == (given_func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE)) { + if (orig_func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE) { + /* For fake closures, scope and name must match. */ + same_closure = orig_func->common.scope == given_func->common.scope + && orig_func->common.function_name == given_func->common.function_name; + } else { + /* Otherwise the opcode structure must be identical. */ + ZEND_ASSERT(orig_func->type == ZEND_USER_FUNCTION); + same_closure = orig_func->op_array.opcodes == given_func->op_array.opcodes; + } } + if (!same_closure) { if (!variadic) { efree(params);