From 6b6c2c003c69729832a7804c76bff6e230b73c91 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 24 Aug 2020 11:47:31 +0200 Subject: [PATCH] Fix #79979: passing value to by-ref param via CUFA crashes If a by-val send is not allowed, we must not do so. Instead we wrap the value in a temporary reference. Closes GH-6000 --- NEWS | 4 ++++ Zend/tests/bug79979.phpt | 17 +++++++++++++++++ Zend/zend_execute_API.c | 10 ++++++++-- Zend/zend_vm_def.h | 16 ++++++++++++++-- Zend/zend_vm_execute.h | 16 ++++++++++++++-- 5 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 Zend/tests/bug79979.phpt diff --git a/NEWS b/NEWS index 4342041a8d..58b0359c28 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2020, PHP 7.4.11 +- Core: + . Fixed bug #79979 (passing value to by-ref param via CUFA crashes). (cmb, + Nikita) + - Calendar: . Fixed bug #80007 (Potential type confusion in unixtojd() parameter parsing). (Andy Postnikov) diff --git a/Zend/tests/bug79979.phpt b/Zend/tests/bug79979.phpt new file mode 100644 index 0000000000..8d7f30b7c8 --- /dev/null +++ b/Zend/tests/bug79979.phpt @@ -0,0 +1,17 @@ +--TEST-- +Bug #79979 (passing value to by-ref param via CUF(A) crashes) +--FILE-- + +--EXPECTF-- +Warning: Parameter 4 to str_replace() expected to be a reference, value given in %s on line %d + +Warning: Parameter 4 to str_replace() expected to be a reference, value given in %s on line %d + +Warning: Parameter 4 to str_replace() expected to be a reference, value given in %s on line %d diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index f2881f7054..78a041d6ce 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -757,6 +757,7 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) / for (i=0; iparam_count; i++) { zval *param; zval *arg = &fci->params[i]; + zend_bool must_wrap = 0; if (ARG_SHOULD_BE_SENT_BY_REF(func, i + 1)) { if (UNEXPECTED(!Z_ISREF_P(arg))) { @@ -765,12 +766,13 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) / ZVAL_NEW_REF(arg, arg); } else if (!ARG_MAY_BE_SENT_BY_REF(func, i + 1)) { /* By-value send is not allowed -- emit a warning, - * but still perform the call with a by-value send. */ + * and perform the call with the value wrapped in a reference. */ zend_error(E_WARNING, "Parameter %d to %s%s%s() expected to be a reference, value given", i+1, func->common.scope ? ZSTR_VAL(func->common.scope->name) : "", func->common.scope ? "::" : "", ZSTR_VAL(func->common.function_name)); + must_wrap = 1; if (UNEXPECTED(EG(exception))) { ZEND_CALL_NUM_ARGS(call) = i; zend_vm_stack_free_args(call); @@ -791,7 +793,11 @@ int zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_cache) / } param = ZEND_CALL_ARG(call, i+1); - ZVAL_COPY(param, arg); + if (EXPECTED(!must_wrap)) { + ZVAL_COPY(param, arg); + } else { + ZVAL_NEW_REF(param, arg); + } } if (UNEXPECTED(func->op_array.fn_flags & ZEND_ACC_CLOSURE)) { diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index a5a2070b43..fa70bf9f96 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -5128,6 +5128,7 @@ ZEND_VM_C_LABEL(send_array): arg_num = 1; param = ZEND_CALL_ARG(EX(call), 1); ZEND_HASH_FOREACH_VAL(ht, arg) { + zend_bool must_wrap = 0; if (skip > 0) { skip--; continue; @@ -5139,6 +5140,7 @@ ZEND_VM_C_LABEL(send_array): /* By-value send is not allowed -- emit a warning, * but still perform the call. */ zend_param_must_be_ref(EX(call)->func, arg_num); + must_wrap = 1; } } } else { @@ -5148,7 +5150,11 @@ ZEND_VM_C_LABEL(send_array): arg = Z_REFVAL_P(arg); } } - ZVAL_COPY(param, arg); + if (EXPECTED(!must_wrap)) { + ZVAL_COPY(param, arg); + } else { + ZVAL_NEW_REF(param, arg); + } ZEND_CALL_NUM_ARGS(EX(call))++; arg_num++; param++; @@ -5160,12 +5166,14 @@ ZEND_VM_C_LABEL(send_array): arg_num = 1; param = ZEND_CALL_ARG(EX(call), 1); ZEND_HASH_FOREACH_VAL(ht, arg) { + zend_bool must_wrap = 0; if (ARG_SHOULD_BE_SENT_BY_REF(EX(call)->func, arg_num)) { if (UNEXPECTED(!Z_ISREF_P(arg))) { if (!ARG_MAY_BE_SENT_BY_REF(EX(call)->func, arg_num)) { /* By-value send is not allowed -- emit a warning, * but still perform the call. */ zend_param_must_be_ref(EX(call)->func, arg_num); + must_wrap = 1; } } } else { @@ -5175,7 +5183,11 @@ ZEND_VM_C_LABEL(send_array): arg = Z_REFVAL_P(arg); } } - ZVAL_COPY(param, arg); + if (EXPECTED(!must_wrap)) { + ZVAL_COPY(param, arg); + } else { + ZVAL_NEW_REF(param, arg); + } ZEND_CALL_NUM_ARGS(EX(call))++; arg_num++; param++; diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 6aa34bbd10..02b0ff24d1 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -2072,6 +2072,7 @@ send_array: arg_num = 1; param = ZEND_CALL_ARG(EX(call), 1); ZEND_HASH_FOREACH_VAL(ht, arg) { + zend_bool must_wrap = 0; if (skip > 0) { skip--; continue; @@ -2083,6 +2084,7 @@ send_array: /* By-value send is not allowed -- emit a warning, * but still perform the call. */ zend_param_must_be_ref(EX(call)->func, arg_num); + must_wrap = 1; } } } else { @@ -2092,7 +2094,11 @@ send_array: arg = Z_REFVAL_P(arg); } } - ZVAL_COPY(param, arg); + if (EXPECTED(!must_wrap)) { + ZVAL_COPY(param, arg); + } else { + ZVAL_NEW_REF(param, arg); + } ZEND_CALL_NUM_ARGS(EX(call))++; arg_num++; param++; @@ -2104,12 +2110,14 @@ send_array: arg_num = 1; param = ZEND_CALL_ARG(EX(call), 1); ZEND_HASH_FOREACH_VAL(ht, arg) { + zend_bool must_wrap = 0; if (ARG_SHOULD_BE_SENT_BY_REF(EX(call)->func, arg_num)) { if (UNEXPECTED(!Z_ISREF_P(arg))) { if (!ARG_MAY_BE_SENT_BY_REF(EX(call)->func, arg_num)) { /* By-value send is not allowed -- emit a warning, * but still perform the call. */ zend_param_must_be_ref(EX(call)->func, arg_num); + must_wrap = 1; } } } else { @@ -2119,7 +2127,11 @@ send_array: arg = Z_REFVAL_P(arg); } } - ZVAL_COPY(param, arg); + if (EXPECTED(!must_wrap)) { + ZVAL_COPY(param, arg); + } else { + ZVAL_NEW_REF(param, arg); + } ZEND_CALL_NUM_ARGS(EX(call))++; arg_num++; param++; -- 2.40.0