Fix bug #79108

Don't expose references in debug_backtrace() or exception traces.
This is regardless of whether the argument is by-reference or not.

As a side-effect of this change, exception traces may now acquire
the interior value of a reference, which may be unexpected for
some internal functions. This is what necessitated the change in
the spl_array sort implementation.

diff --git a/NEWS b/NEWS
index a17f4c0919d308b3fced11eeacc67821552b3633..591655bd107d0e2619cc917ad35cfbae88f479f8 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ PHP                                                                        NEWS
 - Core:
   . Fixed bug #78236 (convert error on receiving variables when duplicate [).
     (cmb)
+  . Fixed bug #79108 (Referencing argument in a function makes it a reference
+    in the stack trace). (Nikita)
 
 - JIT:
   . Fixed bug #79864 (JIT segfault in Symfony OptionsResolver). (Dmitry)

diff --git a/UPGRADING b/UPGRADING
index a98aeab045b126979939f8324cc202c46dea7004..f7c6ec0a147ed114a6da73df0db4f82d314af8c4 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -210,6 +210,9 @@ PHP 8.0 UPGRADE NOTES
     RFC: https://wiki.php.net/rfc/namespaced_names_as_token
   . Nested ternaries now require explicit parentheses.
     RFC: https://wiki.php.net/rfc/ternary_associativity
+  . debug_backtrace() and Exception::getTrace() will no longer provide
+    references to arguments. It will not be possible to change function
+    arguments through the backtrace.
 
 - COM:
   . Removed the ability to import case-insensitive constants from type

diff --git a/Zend/tests/bug70547.phpt b/Zend/tests/bug70547.phpt
index d185cbc3a6f987f21b79e30cf9ddf99bdba58767..e8ebfe9dc9e0941a7495192d61313a2c294294ba 100644 (file)
--- a/Zend/tests/bug70547.phpt
+++ b/Zend/tests/bug70547.phpt
@@ -33,7 +33,7 @@ array(4) {
   [0]=>
   string(3) "1st"
   [1]=>
-  &string(3) "2nd"
+  string(3) "2nd"
   [2]=>
   string(3) "3th"
   [3]=>
@@ -57,7 +57,7 @@ array(4) {
   [0]=>
   string(3) "1st"
   [1]=>
-  &string(3) "2nd"
+  string(3) "2nd"
   [2]=>
   NULL
   [3]=>
@@ -77,7 +77,7 @@ array(4) {
   [0]=>
   NULL
   [1]=>
-  &string(3) "2nd"
+  string(3) "2nd"
   [2]=>
   NULL
   [3]=>

diff --git a/Zend/tests/bug79108.phpt b/Zend/tests/bug79108.phpt
new file mode 100644 (file)
index 0000000..9d1b7c9
--- /dev/null
+++ b/Zend/tests/bug79108.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #79108: Referencing argument in a function makes it a reference in the stack trace
+--FILE--
+<?php
+
+function test(string $val) {
+    $a = &$val;
+    hackingHere();
+    print_r($val);
+}
+
+function hackingHere() {
+    // we're able to modify the $val from here, even though the arg was not a reference
+    debug_backtrace()[1]['args'][0] = 'Modified';
+}
+
+test('Original');
+
+?>
+--EXPECT--
+Original

diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c
index c75f2f465bf8c4065f6b350f68466593a6803f2f..1de771660aae932d13967f466586c65f16f222b3 100644 (file)
--- a/Zend/zend_builtin_functions.c
+++ b/Zend/zend_builtin_functions.c
@@ -1591,16 +1591,12 @@ static void debug_backtrace_get_args(zend_execute_data *call, zval *arg_array) /
                                         * and we have to access them through symbol_table
                                         * See: https://bugs.php.net/bug.php?id=73156
                                         */
-                                       zend_string *arg_name;
-                                       zval *arg;
-
                                        while (i < first_extra_arg) {
-                                               arg_name = call->func->op_array.vars[i];
-                                               arg = zend_hash_find_ex_ind(call->symbol_table, arg_name, 1);
+                                               zend_string *arg_name = call->func->op_array.vars[i];
+                                               zval *arg = zend_hash_find_ex_ind(call->symbol_table, arg_name, 1);
                                                if (arg) {
-                                                       if (Z_OPT_REFCOUNTED_P(arg)) {
-                                                               Z_ADDREF_P(arg);
-                                                       }
+                                                       ZVAL_DEREF(arg);
+                                                       Z_TRY_ADDREF_P(arg);
                                                        ZEND_HASH_FILL_SET(arg);
                                                } else {
                                                        ZEND_HASH_FILL_SET_NULL();
@@ -1611,10 +1607,10 @@ static void debug_backtrace_get_args(zend_execute_data *call, zval *arg_array) /
                                } else {
                                        while (i < first_extra_arg) {
                                                if (EXPECTED(Z_TYPE_INFO_P(p) != IS_UNDEF)) {
-                                                       if (Z_OPT_REFCOUNTED_P(p)) {
-                                                               Z_ADDREF_P(p);
-                                                       }
-                                                       ZEND_HASH_FILL_SET(p);
+                                                       zval *arg = p;
+                                                       ZVAL_DEREF(arg);
+                                                       Z_TRY_ADDREF_P(arg);
+                                                       ZEND_HASH_FILL_SET(arg);
                                                } else {
                                                        ZEND_HASH_FILL_SET_NULL();
                                                }
@@ -1628,10 +1624,10 @@ static void debug_backtrace_get_args(zend_execute_data *call, zval *arg_array) /
 
                        while (i < num_args) {
                                if (EXPECTED(Z_TYPE_INFO_P(p) != IS_UNDEF)) {
-                                       if (Z_OPT_REFCOUNTED_P(p)) {
-                                               Z_ADDREF_P(p);
-                                       }
-                                       ZEND_HASH_FILL_SET(p);
+                                       zval *arg = p;
+                                       ZVAL_DEREF(arg);
+                                       Z_TRY_ADDREF_P(arg);
+                                       ZEND_HASH_FILL_SET(arg);
                                } else {
                                        ZEND_HASH_FILL_SET_NULL();
                                }

diff --git a/ext/mbstring/tests/mb_ereg1.phpt b/ext/mbstring/tests/mb_ereg1.phpt
index 27e321bb64036563e37ac0d3788cefd9242daa5b..5fa830fa63bfa13bcd03ba81ee0b5cefd7d17697 100644 (file)
--- a/ext/mbstring/tests/mb_ereg1.phpt
+++ b/ext/mbstring/tests/mb_ereg1.phpt
@@ -53,7 +53,7 @@ array(3) {
   [1]=>
   int(1)
   [2]=>
-  &string(0) ""
+  string(0) ""
 }
 mb_ereg(): Argument #2 ($string) must be of type string, array given
 array(3) {
@@ -63,7 +63,7 @@ array(3) {
   array(0) {
   }
   [2]=>
-  &string(0) ""
+  string(0) ""
 }
 bool(false)
 array(3) {

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index a2dfff3482223365ad9c804af25380c11196553e..69d79a7501a85eaee48dc9ac91a8083c6a6645d8 100644 (file)
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -111,13 +111,6 @@ static inline HashTable *spl_array_get_hash_table(spl_array_object* intern) { /*
 }
 /* }}} */
 
-static inline void spl_array_replace_hash_table(spl_array_object* intern, HashTable *ht) { /* {{{ */
-       HashTable **ht_ptr = spl_array_get_hash_table_ptr(intern);
-       zend_array_destroy(*ht_ptr);
-       *ht_ptr = ht;
-}
-/* }}} */
-
 static inline zend_bool spl_array_is_object(spl_array_object *intern) /* {{{ */
 {
        while (intern->ar_flags & SPL_ARRAY_USE_OTHER) {
@@ -1412,7 +1405,8 @@ PHP_METHOD(ArrayObject, count)
 static void spl_array_method(INTERNAL_FUNCTION_PARAMETERS, char *fname, int fname_len, int use_arg) /* {{{ */
 {
        spl_array_object *intern = Z_SPLARRAY_P(ZEND_THIS);
-       HashTable *aht = spl_array_get_hash_table(intern);
+       HashTable **ht_ptr = spl_array_get_hash_table_ptr(intern);
+       HashTable *aht = *ht_ptr;
        zval function_name, params[2], *arg = NULL;
 
        ZVAL_STRINGL(&function_name, fname, fname_len);
@@ -1451,13 +1445,11 @@ static void spl_array_method(INTERNAL_FUNCTION_PARAMETERS, char *fname, int fnam
 
 exit:
        {
-               HashTable *new_ht = Z_ARRVAL_P(Z_REFVAL(params[0]));
-               if (aht != new_ht) {
-                       spl_array_replace_hash_table(intern, new_ht);
-               } else {
-                       GC_DELREF(aht);
-               }
-               ZVAL_NULL(Z_REFVAL(params[0]));
+               zval *ht_zv = Z_REFVAL(params[0]);
+               zend_array_release(*ht_ptr);
+               SEPARATE_ARRAY(ht_zv);
+               *ht_ptr = Z_ARRVAL_P(ht_zv);
+               ZVAL_NULL(ht_zv);
                zval_ptr_dtor(&params[0]);
                zend_string_free(Z_STR(function_name));
        }

diff --git a/ext/standard/tests/array/array_walk_closure.phpt b/ext/standard/tests/array/array_walk_closure.phpt
index 68e56568e7c782041759a2f55ae0d4cad2fa11b1..aab0002e466028a8182b5d47a91f7ea6236f2a7b 100644 (file)
--- a/ext/standard/tests/array/array_walk_closure.phpt
+++ b/ext/standard/tests/array/array_walk_closure.phpt
@@ -223,7 +223,7 @@ array(2) {
     ["args"]=>
     array(2) {
       [0]=>
-      &array(3) {
+      array(3) {
         ["one"]=>
         int(1)
         ["two"]=>

diff --git a/ext/standard/tests/array/bug79839.phpt b/ext/standard/tests/array/bug79839.phpt
index 901be9c8def9e8aacbebd2b7e493eb4ffb0b4f2a..643604cb9b20a4e55285f789eadeafade8dcb232 100644 (file)
--- a/ext/standard/tests/array/bug79839.phpt
+++ b/ext/standard/tests/array/bug79839.phpt
@@ -22,5 +22,5 @@ var_dump($test);
 Cannot assign array to reference held by property Test::$prop of type int
 object(Test)#1 (1) {
   ["prop"]=>
-  &int(42)
+  int(42)
 }