diff --git a/NEWS b/NEWS index 4e1196fb09466..482dfcb091342 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). diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 587bca11522b2..2692e1928068d 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 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); diff --git a/ext/reflection/tests/gh21362.phpt b/ext/reflection/tests/gh21362.phpt new file mode 100644 index 0000000000000..ce7840b494ed8 --- /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 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)