mirror of
https://github.com/php/php-src.git
synced 2026-03-24 00:02:20 +01:00
Fix GH-19653: Closure named argument unpacking between temporary closures can cause a crash
Due to user closures, the `fbc` address isn't unique if the memory address is reused. We need to distinguish using a unique key, and we choose arg_info such that it can be reused across different functions. Closes GH-19654.
This commit is contained in:
2
NEWS
2
NEWS
@@ -8,6 +8,8 @@ PHP NEWS
|
||||
. Fixed hard_timeout with --enable-zend-max-execution-timers. (Appla)
|
||||
. Fixed bug GH-19792 (SCCP causes UAF for return value if both warning and
|
||||
exception are triggered). (nielsdos)
|
||||
. Fixed bug GH-19653 (Closure named argument unpacking between temporary
|
||||
closures can cause a crash). (nielsdos, Arnaud, Bob)
|
||||
|
||||
- Curl:
|
||||
. Fix cloning of CURLOPT_POSTFIELDS when using the clone operator instead
|
||||
|
||||
27
Zend/tests/closures/gh19653_1.phpt
Normal file
27
Zend/tests/closures/gh19653_1.phpt
Normal file
@@ -0,0 +1,27 @@
|
||||
--TEST--
|
||||
GH-19653 (Closure named argument unpacking between temporary closures can cause a crash)
|
||||
--CREDITS--
|
||||
ivan-u7n
|
||||
--FILE--
|
||||
<?php
|
||||
|
||||
function func1(string $a1 = 'a1', string $a2 = 'a2', string $a3 = 'a3') {
|
||||
echo __FUNCTION__ . "() a1=$a1 a2=$a2 a3=$a3\n";
|
||||
}
|
||||
|
||||
function usage1(?Closure $func1 = null) {
|
||||
echo __FUNCTION__ . "() ";
|
||||
($func1 ?? func1(...))(a3: 'm3+');
|
||||
}
|
||||
usage1();
|
||||
|
||||
$func1 = function (string ...$args) {
|
||||
echo "[function] ";
|
||||
func1(...$args, a2: 'm2+');
|
||||
};
|
||||
usage1(func1: $func1);
|
||||
|
||||
?>
|
||||
--EXPECT--
|
||||
usage1() func1() a1=a1 a2=a2 a3=m3+
|
||||
usage1() [function] func1() a1=a1 a2=m2+ a3=m3+
|
||||
23
Zend/tests/closures/gh19653_2.phpt
Normal file
23
Zend/tests/closures/gh19653_2.phpt
Normal file
@@ -0,0 +1,23 @@
|
||||
--TEST--
|
||||
GH-19653 (Closure named argument unpacking between temporary closures can cause a crash) - eval variation
|
||||
--CREDITS--
|
||||
arnaud-lb
|
||||
--FILE--
|
||||
<?php
|
||||
|
||||
function usage1(Closure $c) {
|
||||
$c(a: 1);
|
||||
}
|
||||
|
||||
usage1(eval('return function($a) { var_dump($a); };'));
|
||||
usage1(eval('return function($b) { var_dump($b); };'));
|
||||
|
||||
?>
|
||||
--EXPECTF--
|
||||
int(1)
|
||||
|
||||
Fatal error: Uncaught Error: Unknown named parameter $a in %s:%d
|
||||
Stack trace:
|
||||
#0 %s(%d): usage1(Object(Closure))
|
||||
#1 {main}
|
||||
thrown in %s on line %d
|
||||
@@ -5058,7 +5058,12 @@ static zend_never_inline zend_result ZEND_FASTCALL zend_quick_check_constant(
|
||||
|
||||
static zend_always_inline uint32_t zend_get_arg_offset_by_name(
|
||||
zend_function *fbc, zend_string *arg_name, void **cache_slot) {
|
||||
if (EXPECTED(*cache_slot == fbc)) {
|
||||
/* Due to closures, the `fbc` address isn't unique if the memory address is reused.
|
||||
* The argument info will be however and uniquely positions the arguments.
|
||||
* We do support NULL arg_info, so we have to distinguish that from an uninitialized cache slot. */
|
||||
void *unique_id = (void *) ((uintptr_t) fbc->common.arg_info | 1);
|
||||
|
||||
if (EXPECTED(*cache_slot == unique_id)) {
|
||||
return *(uintptr_t *)(cache_slot + 1);
|
||||
}
|
||||
|
||||
@@ -5069,8 +5074,10 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
|
||||
for (uint32_t i = 0; i < num_args; i++) {
|
||||
zend_arg_info *arg_info = &fbc->op_array.arg_info[i];
|
||||
if (zend_string_equals(arg_name, arg_info->name)) {
|
||||
*cache_slot = fbc;
|
||||
*(uintptr_t *)(cache_slot + 1) = i;
|
||||
if (!fbc->op_array.refcount || !(fbc->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
|
||||
*cache_slot = unique_id;
|
||||
*(uintptr_t *)(cache_slot + 1) = i;
|
||||
}
|
||||
return i;
|
||||
}
|
||||
}
|
||||
@@ -5079,7 +5086,7 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
|
||||
zend_internal_arg_info *arg_info = &fbc->internal_function.arg_info[i];
|
||||
size_t len = strlen(arg_info->name);
|
||||
if (zend_string_equals_cstr(arg_name, arg_info->name, len)) {
|
||||
*cache_slot = fbc;
|
||||
*cache_slot = unique_id;
|
||||
*(uintptr_t *)(cache_slot + 1) = i;
|
||||
return i;
|
||||
}
|
||||
@@ -5087,8 +5094,10 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
|
||||
}
|
||||
|
||||
if (fbc->common.fn_flags & ZEND_ACC_VARIADIC) {
|
||||
*cache_slot = fbc;
|
||||
*(uintptr_t *)(cache_slot + 1) = fbc->common.num_args;
|
||||
if (fbc->type == ZEND_INTERNAL_FUNCTION || !fbc->op_array.refcount || !(fbc->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
|
||||
*cache_slot = unique_id;
|
||||
*(uintptr_t *)(cache_slot + 1) = fbc->common.num_args;
|
||||
}
|
||||
return fbc->common.num_args;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user