Skip to content

Commit a3d27aa

Browse files
committed
Merge branch 'PHP-8.3' into PHP-8.4
* PHP-8.3: Fix phpGH-19653: Closure named argument unpacking between temporary closures can cause a crash
2 parents 8774e96 + 2225295 commit a3d27aa

File tree

4 files changed

+67
-6
lines changed

4 files changed

+67
-6
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ PHP NEWS
88
. Fixed hard_timeout with --enable-zend-max-execution-timers. (Appla)
99
. Fixed bug GH-19792 (SCCP causes UAF for return value if both warning and
1010
exception are triggered). (nielsdos)
11+
. Fixed bug GH-19653 (Closure named argument unpacking between temporary
12+
closures can cause a crash). (nielsdos, Arnaud, Bob)
1113

1214
- Curl:
1315
. Fix cloning of CURLOPT_POSTFIELDS when using the clone operator instead

Zend/tests/closures/gh19653_1.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-19653 (Closure named argument unpacking between temporary closures can cause a crash)
3+
--CREDITS--
4+
ivan-u7n
5+
--FILE--
6+
<?php
7+
8+
function func1(string $a1 = 'a1', string $a2 = 'a2', string $a3 = 'a3') {
9+
echo __FUNCTION__ . "() a1=$a1 a2=$a2 a3=$a3\n";
10+
}
11+
12+
function usage1(?Closure $func1 = null) {
13+
echo __FUNCTION__ . "() ";
14+
($func1 ?? func1(...))(a3: 'm3+');
15+
}
16+
usage1();
17+
18+
$func1 = function (string ...$args) {
19+
echo "[function] ";
20+
func1(...$args, a2: 'm2+');
21+
};
22+
usage1(func1: $func1);
23+
24+
?>
25+
--EXPECT--
26+
usage1() func1() a1=a1 a2=a2 a3=m3+
27+
usage1() [function] func1() a1=a1 a2=m2+ a3=m3+

Zend/tests/closures/gh19653_2.phpt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
GH-19653 (Closure named argument unpacking between temporary closures can cause a crash) - eval variation
3+
--CREDITS--
4+
arnaud-lb
5+
--FILE--
6+
<?php
7+
8+
function usage1(Closure $c) {
9+
$c(a: 1);
10+
}
11+
12+
usage1(eval('return function($a) { var_dump($a); };'));
13+
usage1(eval('return function($b) { var_dump($b); };'));
14+
15+
?>
16+
--EXPECTF--
17+
int(1)
18+
19+
Fatal error: Uncaught Error: Unknown named parameter $a in %s:%d
20+
Stack trace:
21+
#0 %s(%d): usage1(Object(Closure))
22+
#1 {main}
23+
thrown in %s on line %d

Zend/zend_execute.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5250,7 +5250,12 @@ static zend_never_inline zend_result ZEND_FASTCALL zend_quick_check_constant(
52505250

52515251
static zend_always_inline uint32_t zend_get_arg_offset_by_name(
52525252
zend_function *fbc, zend_string *arg_name, void **cache_slot) {
5253-
if (EXPECTED(*cache_slot == fbc)) {
5253+
/* Due to closures, the `fbc` address isn't unique if the memory address is reused.
5254+
* The argument info will be however and uniquely positions the arguments.
5255+
* We do support NULL arg_info, so we have to distinguish that from an uninitialized cache slot. */
5256+
void *unique_id = (void *) ((uintptr_t) fbc->common.arg_info | 1);
5257+
5258+
if (EXPECTED(*cache_slot == unique_id)) {
52545259
return *(uintptr_t *)(cache_slot + 1);
52555260
}
52565261

@@ -5261,8 +5266,10 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
52615266
for (uint32_t i = 0; i < num_args; i++) {
52625267
zend_arg_info *arg_info = &fbc->op_array.arg_info[i];
52635268
if (zend_string_equals(arg_name, arg_info->name)) {
5264-
*cache_slot = fbc;
5265-
*(uintptr_t *)(cache_slot + 1) = i;
5269+
if (!fbc->op_array.refcount || !(fbc->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
5270+
*cache_slot = unique_id;
5271+
*(uintptr_t *)(cache_slot + 1) = i;
5272+
}
52665273
return i;
52675274
}
52685275
}
@@ -5271,16 +5278,18 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
52715278
zend_internal_arg_info *arg_info = &fbc->internal_function.arg_info[i];
52725279
size_t len = strlen(arg_info->name);
52735280
if (zend_string_equals_cstr(arg_name, arg_info->name, len)) {
5274-
*cache_slot = fbc;
5281+
*cache_slot = unique_id;
52755282
*(uintptr_t *)(cache_slot + 1) = i;
52765283
return i;
52775284
}
52785285
}
52795286
}
52805287

52815288
if (fbc->common.fn_flags & ZEND_ACC_VARIADIC) {
5282-
*cache_slot = fbc;
5283-
*(uintptr_t *)(cache_slot + 1) = fbc->common.num_args;
5289+
if (fbc->type == ZEND_INTERNAL_FUNCTION || !fbc->op_array.refcount || !(fbc->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
5290+
*cache_slot = unique_id;
5291+
*(uintptr_t *)(cache_slot + 1) = fbc->common.num_args;
5292+
}
52845293
return fbc->common.num_args;
52855294
}
52865295

0 commit comments

Comments
 (0)