diff options
author | Nikita Popov <nikita.ppv@gmail.com> | 2020-07-24 10:16:38 +0200 |
---|---|---|
committer | Nikita Popov <nikita.ppv@gmail.com> | 2020-07-24 12:23:34 +0200 |
commit | d65d3f5298dcc8d94bcac96e8bf2441dceb393ac (patch) | |
tree | a5e07c4bdf5a7ca0dc213b1b3a7ef6e0136d8528 | |
parent | 27ad19c3e8d4fe61ce9c8cec9e50062acf2255c1 (diff) | |
download | php-git-d65d3f5298dcc8d94bcac96e8bf2441dceb393ac.tar.gz |
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.
-rw-r--r-- | NEWS | 2 | ||||
-rw-r--r-- | UPGRADING | 3 | ||||
-rw-r--r-- | Zend/tests/bug70547.phpt | 6 | ||||
-rw-r--r-- | Zend/tests/bug79108.phpt | 21 | ||||
-rw-r--r-- | Zend/zend_builtin_functions.c | 28 | ||||
-rw-r--r-- | ext/mbstring/tests/mb_ereg1.phpt | 4 | ||||
-rw-r--r-- | ext/spl/spl_array.c | 22 | ||||
-rw-r--r-- | ext/standard/tests/array/array_walk_closure.phpt | 2 | ||||
-rw-r--r-- | ext/standard/tests/array/bug79839.phpt | 2 |
9 files changed, 52 insertions, 38 deletions
@@ -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) @@ -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 d185cbc3a6..e8ebfe9dc9 100644 --- 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 index 0000000000..9d1b7c963b --- /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 c75f2f465b..1de771660a 100644 --- 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 27e321bb64..5fa830fa63 100644 --- 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 a2dfff3482..69d79a7501 100644 --- 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(¶ms[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 68e56568e7..aab0002e46 100644 --- 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 901be9c8de..643604cb9b 100644 --- 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) } |