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 b5ac3d68781bf8a4a959c7a7bcec9b15f2a93254..be9592d19353ed1cd17d7f8c2bb11c7bd4b66036 100644 (file)
--- 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 74ed8a8d173b3973da663b8002ebf36b569e3c97..5ffe569e4a615a000df6fcf03804c9d3bf2abf80 100644 (file)
--- 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 (file)
index 0000000..f0f743e
--- /dev/null
+++ b/Zend/tests/bug76047.phpt
@@ -0,0 +1,68 @@
+--TEST--
+Bug #76047: Use-after-free when accessing already destructed backtrace arguments
+--FILE--
+<?php
+
+class Vuln {
+    public $a;
+    public function __destruct() {
+        unset($this->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 62a9ad8bed8178c3020915a8234db68b5c8bbaf5..fbd79019e762987e5b4d699d4cba11ce5bf3df9f 100644 (file)
--- 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 e893e35b6d53f4b51875e02d53ae969b88b2a254..33518478c32e9731549a7e4b04935f83b95ec50f 100644 (file)
--- 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)));
                        }