From 96bf925cde2af891e9fca761ffecbfebef6a0d30 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 2 Jul 2021 11:15:31 +0200 Subject: [PATCH] Fix return value of wrong fucntion by-ref assign We should be using the result of zend_assign_to_variable() here, which will deref prior to potential freeing. Fixes oss-fuzz #29899. --- ...n_by_val_function_by_ref_return_value.phpt | 11 ++++++++++ Zend/zend_execute.c | 14 +++++-------- Zend/zend_vm_def.h | 5 ++--- Zend/zend_vm_execute.h | 20 ++++++++----------- 4 files changed, 26 insertions(+), 24 deletions(-) create mode 100644 Zend/tests/assign_by_val_function_by_ref_return_value.phpt diff --git a/Zend/tests/assign_by_val_function_by_ref_return_value.phpt b/Zend/tests/assign_by_val_function_by_ref_return_value.phpt new file mode 100644 index 00000000000..64905d10f1f --- /dev/null +++ b/Zend/tests/assign_by_val_function_by_ref_return_value.phpt @@ -0,0 +1,11 @@ +--TEST-- +Return value of assigning by-val function result by-reference +--FILE-- + +--EXPECTF-- +Notice: Only variables should be assigned by reference in %s on line %d +NULL diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 0c9a85bff50..35cf24e5868 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -585,18 +585,16 @@ static zend_never_inline zval* zend_assign_to_typed_property_reference(zend_prop return prop; } -static zend_never_inline ZEND_COLD int zend_wrong_assign_to_variable_reference(zval *variable_ptr, zval *value_ptr OPLINE_DC EXECUTE_DATA_DC) +static zend_never_inline ZEND_COLD zval *zend_wrong_assign_to_variable_reference(zval *variable_ptr, zval *value_ptr OPLINE_DC EXECUTE_DATA_DC) { zend_error(E_NOTICE, "Only variables should be assigned by reference"); if (UNEXPECTED(EG(exception) != NULL)) { - return 0; + return &EG(uninitialized_zval); } /* Use IS_TMP_VAR instead of IS_VAR to avoid ISREF check */ Z_TRY_ADDREF_P(value_ptr); - value_ptr = zend_assign_to_variable(variable_ptr, value_ptr, IS_TMP_VAR, EX_USES_STRICT_TYPES()); - - return 1; + return zend_assign_to_variable(variable_ptr, value_ptr, IS_TMP_VAR, EX_USES_STRICT_TYPES()); } static void zend_format_type(zend_type type, const char **part1, const char **part2) { @@ -2948,10 +2946,8 @@ static zend_always_inline void zend_assign_to_property_reference(zval *container (opline->extended_value & ZEND_RETURNS_FUNCTION) && UNEXPECTED(!Z_ISREF_P(value_ptr))) { - if (UNEXPECTED(!zend_wrong_assign_to_variable_reference( - variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) { - variable_ptr = &EG(uninitialized_zval); - } + variable_ptr = zend_wrong_assign_to_variable_reference( + variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC); } else { zend_property_info *prop_info = NULL; diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index d693f4f3599..b69563a0228 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2772,9 +2772,8 @@ ZEND_VM_HANDLER(30, ZEND_ASSIGN_REF, VAR|CV, VAR|CV, SRC) opline->extended_value == ZEND_RETURNS_FUNCTION && UNEXPECTED(!Z_ISREF_P(value_ptr))) { - if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) { - variable_ptr = &EG(uninitialized_zval); - } + variable_ptr = zend_wrong_assign_to_variable_reference( + variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC); } else { zend_assign_to_variable_reference(variable_ptr, value_ptr); } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index dc49bcd4342..fd807054437 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -26766,9 +26766,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_VAR_VAR_HANDLE opline->extended_value == ZEND_RETURNS_FUNCTION && UNEXPECTED(!Z_ISREF_P(value_ptr))) { - if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) { - variable_ptr = &EG(uninitialized_zval); - } + variable_ptr = zend_wrong_assign_to_variable_reference( + variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC); } else { zend_assign_to_variable_reference(variable_ptr, value_ptr); } @@ -29698,9 +29697,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_VAR_CV_HANDLER opline->extended_value == ZEND_RETURNS_FUNCTION && UNEXPECTED(!Z_ISREF_P(value_ptr))) { - if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) { - variable_ptr = &EG(uninitialized_zval); - } + variable_ptr = zend_wrong_assign_to_variable_reference( + variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC); } else { zend_assign_to_variable_reference(variable_ptr, value_ptr); } @@ -45451,9 +45449,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_CV_VAR_HANDLER opline->extended_value == ZEND_RETURNS_FUNCTION && UNEXPECTED(!Z_ISREF_P(value_ptr))) { - if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) { - variable_ptr = &EG(uninitialized_zval); - } + variable_ptr = zend_wrong_assign_to_variable_reference( + variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC); } else { zend_assign_to_variable_reference(variable_ptr, value_ptr); } @@ -49507,9 +49504,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_CV_CV_HANDLER( opline->extended_value == ZEND_RETURNS_FUNCTION && UNEXPECTED(!Z_ISREF_P(value_ptr))) { - if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) { - variable_ptr = &EG(uninitialized_zval); - } + variable_ptr = zend_wrong_assign_to_variable_reference( + variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC); } else { zend_assign_to_variable_reference(variable_ptr, value_ptr); }