summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNikita Popov <nikita.ppv@gmail.com>2020-07-24 10:16:38 +0200
committerNikita Popov <nikita.ppv@gmail.com>2020-07-24 12:23:34 +0200
commitd65d3f5298dcc8d94bcac96e8bf2441dceb393ac (patch)
treea5e07c4bdf5a7ca0dc213b1b3a7ef6e0136d8528
parent27ad19c3e8d4fe61ce9c8cec9e50062acf2255c1 (diff)
downloadphp-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--NEWS2
-rw-r--r--UPGRADING3
-rw-r--r--Zend/tests/bug70547.phpt6
-rw-r--r--Zend/tests/bug79108.phpt21
-rw-r--r--Zend/zend_builtin_functions.c28
-rw-r--r--ext/mbstring/tests/mb_ereg1.phpt4
-rw-r--r--ext/spl/spl_array.c22
-rw-r--r--ext/standard/tests/array/array_walk_closure.phpt2
-rw-r--r--ext/standard/tests/array/bug79839.phpt2
9 files changed, 52 insertions, 38 deletions
diff --git a/NEWS b/NEWS
index a17f4c0919..591655bd10 100644
--- 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 a98aeab045..f7c6ec0a14 100644
--- 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 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(&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 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)
}