mirror of
https://github.com/php/php-src.git
synced 2026-03-24 00:02:20 +01:00
Fix GH-21362: ReflectionMethod::invoke() allows different Closures (#21366)
ReflectionMethod::invoke() (and invokeArgs()) 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
This commit is contained in:
2
NEWS
2
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).
|
||||
|
||||
@@ -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 Closure is not the same as the reflected Closure");
|
||||
RETURN_THROWS();
|
||||
}
|
||||
}
|
||||
}
|
||||
/* Copy the zend_function when calling via handler (e.g. Closure::__invoke()) */
|
||||
callback = _copy_function(mptr);
|
||||
|
||||
53
ext/reflection/tests/gh21362.phpt
Normal file
53
ext/reflection/tests/gh21362.phpt
Normal file
@@ -0,0 +1,53 @@
|
||||
--TEST--
|
||||
GH-21362 (ReflectionMethod::invokeArgs() for Closure::__invoke() accepts objects from different Closures)
|
||||
--FILE--
|
||||
<?php
|
||||
|
||||
$c1 = function ($foo, $bar) {
|
||||
echo "c1: foo={$foo}, bar={$bar}\n";
|
||||
};
|
||||
|
||||
$c2 = function ($bar, $foo) {
|
||||
echo "c2: foo={$foo}, bar={$bar}\n";
|
||||
};
|
||||
|
||||
$m = new ReflectionMethod($c1, '__invoke');
|
||||
|
||||
// invokeArgs with the correct Closure should work
|
||||
$m->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 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)
|
||||
Reference in New Issue
Block a user