From: Nikita Popov Date: Fri, 31 Jan 2020 09:21:37 +0000 (+0100) Subject: Fix bug #76047 X-Git-Tag: php-7.3.15RC1~5 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ef1e4891b47949c8dc0f9482eef9454a0ecdfa1d;p=php Fix bug #76047 Unlink the current stack frame before freeing CVs or extra args. This means it will no longer show up in back traces that are generated during CV destruction. We already did this prior to destructing the object/closure, presumably for the same reason. --- diff --git a/NEWS b/NEWS index b5ac3d6878..be9592d193 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ PHP NEWS supported). (Nikita) . Fixed bug ##79146 (cscript can fail to run on some systems). (clarodeus) . Fixed bug #78323 (Code 0 is returned on invalid options). (Ivan Mikheykin) + . Fixed bug #76047 (Use-after-free when accessing already destructed + backtrace arguments). (Nikita) - CURL: . Fixed bug #79078 (Hypothetical use-after-free in curl_multi_add_handle()). diff --git a/Zend/tests/bug52361.phpt b/Zend/tests/bug52361.phpt index 74ed8a8d17..5ffe569e4a 100644 --- a/Zend/tests/bug52361.phpt +++ b/Zend/tests/bug52361.phpt @@ -25,9 +25,8 @@ try { --EXPECTF-- 1. Exception: aaa in %sbug52361.php:5 Stack trace: -#0 %sbug52361.php(13): aaa->__destruct() -#1 %sbug52361.php(16): bbb() -#2 {main} +#0 %sbug52361.php(16): aaa->__destruct() +#1 {main} 2. Exception: bbb in %sbug52361.php:13 Stack trace: #0 %sbug52361.php(16): bbb() diff --git a/Zend/tests/bug76047.phpt b/Zend/tests/bug76047.phpt new file mode 100644 index 0000000000..f0f743ee3d --- /dev/null +++ b/Zend/tests/bug76047.phpt @@ -0,0 +1,68 @@ +--TEST-- +Bug #76047: Use-after-free when accessing already destructed backtrace arguments +--FILE-- +a); + $backtrace = (new Exception)->getTrace(); + var_dump($backtrace); + } +} + +function test($arg) { + $arg = str_shuffle(str_repeat('A', 79)); + $vuln = new Vuln(); + $vuln->a = $arg; +} + +function test2($arg) { + $$arg = 1; // Trigger symbol table + $arg = str_shuffle(str_repeat('A', 79)); + $vuln = new Vuln(); + $vuln->a = $arg; +} + +test('x'); +test2('x'); + +?> +--EXPECTF-- +array(1) { + [0]=> + array(6) { + ["file"]=> + string(%d) "%s" + ["line"]=> + int(%d) + ["function"]=> + string(10) "__destruct" + ["class"]=> + string(4) "Vuln" + ["type"]=> + string(2) "->" + ["args"]=> + array(0) { + } + } +} +array(1) { + [0]=> + array(6) { + ["file"]=> + string(%d) "%s" + ["line"]=> + int(%d) + ["function"]=> + string(10) "__destruct" + ["class"]=> + string(4) "Vuln" + ["type"]=> + string(2) "->" + ["args"]=> + array(0) { + } + } +} diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 62a9ad8bed..fbd79019e7 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2398,9 +2398,9 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY) uint32_t call_info = EX_CALL_INFO(); if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP|ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_ALLOCATED)) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); - EG(current_execute_data) = EX(prev_execute_data); if (UNEXPECTED(call_info & ZEND_CALL_RELEASE_THIS)) { zend_object *object = Z_OBJ(execute_data->This); #if 0 @@ -2426,12 +2426,12 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY) LOAD_NEXT_OPLINE(); ZEND_VM_LEAVE(); } else if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP)) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); if (UNEXPECTED(call_info & ZEND_CALL_HAS_SYMBOL_TABLE)) { zend_clean_and_cache_symbol_table(EX(symbol_table)); } - EG(current_execute_data) = EX(prev_execute_data); /* Free extra args before releasing the closure, * as that may free the op_array. */ @@ -2481,6 +2481,7 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY) ZEND_VM_LEAVE(); } else { if (EXPECTED((call_info & ZEND_CALL_CODE) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); if (UNEXPECTED(call_info & (ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS))) { if (UNEXPECTED(call_info & ZEND_CALL_HAS_SYMBOL_TABLE)) { @@ -2488,7 +2489,6 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY) } zend_vm_stack_free_extra_args_ex(call_info, execute_data); } - EG(current_execute_data) = EX(prev_execute_data); if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) { OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func))); } diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index e893e35b6d..33518478c3 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -507,9 +507,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_ uint32_t call_info = EX_CALL_INFO(); if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP|ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_ALLOCATED)) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); - EG(current_execute_data) = EX(prev_execute_data); if (UNEXPECTED(call_info & ZEND_CALL_RELEASE_THIS)) { zend_object *object = Z_OBJ(execute_data->This); #if 0 @@ -535,12 +535,12 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_ LOAD_NEXT_OPLINE(); ZEND_VM_LEAVE(); } else if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP)) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); if (UNEXPECTED(call_info & ZEND_CALL_HAS_SYMBOL_TABLE)) { zend_clean_and_cache_symbol_table(EX(symbol_table)); } - EG(current_execute_data) = EX(prev_execute_data); /* Free extra args before releasing the closure, * as that may free the op_array. */ @@ -590,6 +590,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_ ZEND_VM_LEAVE(); } else { if (EXPECTED((call_info & ZEND_CALL_CODE) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); if (UNEXPECTED(call_info & (ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS))) { if (UNEXPECTED(call_info & ZEND_CALL_HAS_SYMBOL_TABLE)) { @@ -597,7 +598,6 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_ } zend_vm_stack_free_extra_args_ex(call_info, execute_data); } - EG(current_execute_data) = EX(prev_execute_data); if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) { OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func))); } @@ -55379,9 +55379,9 @@ zend_leave_helper_SPEC_LABEL: uint32_t call_info = EX_CALL_INFO(); if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP|ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_ALLOCATED)) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); - EG(current_execute_data) = EX(prev_execute_data); if (UNEXPECTED(call_info & ZEND_CALL_RELEASE_THIS)) { zend_object *object = Z_OBJ(execute_data->This); #if 0 @@ -55407,12 +55407,12 @@ zend_leave_helper_SPEC_LABEL: LOAD_NEXT_OPLINE(); ZEND_VM_LEAVE(); } else if (EXPECTED((call_info & (ZEND_CALL_CODE|ZEND_CALL_TOP)) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); if (UNEXPECTED(call_info & ZEND_CALL_HAS_SYMBOL_TABLE)) { zend_clean_and_cache_symbol_table(EX(symbol_table)); } - EG(current_execute_data) = EX(prev_execute_data); /* Free extra args before releasing the closure, * as that may free the op_array. */ @@ -55462,6 +55462,7 @@ zend_leave_helper_SPEC_LABEL: ZEND_VM_LEAVE(); } else { if (EXPECTED((call_info & ZEND_CALL_CODE) == 0)) { + EG(current_execute_data) = EX(prev_execute_data); i_free_compiled_variables(execute_data); if (UNEXPECTED(call_info & (ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS))) { if (UNEXPECTED(call_info & ZEND_CALL_HAS_SYMBOL_TABLE)) { @@ -55469,7 +55470,6 @@ zend_leave_helper_SPEC_LABEL: } zend_vm_stack_free_extra_args_ex(call_info, execute_data); } - EG(current_execute_data) = EX(prev_execute_data); if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) { OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func))); }