diff --git a/Zend/tests/closures/gh19653_1.phpt b/Zend/tests/closures/gh19653_1.phpt new file mode 100644 index 0000000000000..93b119eb57826 --- /dev/null +++ b/Zend/tests/closures/gh19653_1.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-19653 (Closure named argument unpacking between temporary closures can cause a crash) +--CREDITS-- +ivan-u7n +--FILE-- + +--EXPECT-- +usage1() func1() a1=a1 a2=a2 a3=m3+ +usage1() [function] func1() a1=a1 a2=m2+ a3=m3+ diff --git a/Zend/tests/closures/gh19653_2.phpt b/Zend/tests/closures/gh19653_2.phpt new file mode 100644 index 0000000000000..7eb837dd22c4d --- /dev/null +++ b/Zend/tests/closures/gh19653_2.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-19653 (Closure named argument unpacking between temporary closures can cause a crash) - eval variation +--CREDITS-- +arnaud-lb +--FILE-- + +--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 diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ba9a7fa7e528f..385fe9af0bbfe 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -5440,7 +5440,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); } @@ -5451,8 +5456,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; } } @@ -5462,7 +5469,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; } @@ -5470,8 +5477,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; }