diff --git a/NEWS b/NEWS index 6a0aa043a5e1..73e48f46601d 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,9 @@ PHP NEWS ?? ??? ????, PHP 8.6.0RC1 - Core: + . Fixed bug GH-23725 (use-after-free when __toString() destroys an array or + string argument of a frameless in_array(), preg_replace() or + str_replace() call). (Ilia Alshanetsky) . Fixed incorrect internal pointer and foreach iterator positions when compacting arrays with holes. (Weilin Du) . Fix handling of references to typed properties during unserialization diff --git a/Zend/zend_frameless_function.h b/Zend/zend_frameless_function.h index b6f361f104b0..288210cb78fe 100644 --- a/Zend/zend_frameless_function.h +++ b/Zend/zend_frameless_function.h @@ -43,23 +43,30 @@ #define Z_FLF_PARAM_ZVAL(arg_num, dest) \ dest = arg ## arg_num; -#define Z_FLF_PARAM_ARRAY(arg_num, dest) \ - if (!zend_parse_arg_array(arg ## arg_num, &dest, /* null_check */ false, /* or_object */ false)) { \ +#define Z_FLF_PARAM_ARRAY(arg_num, dest_ht) \ + if (!zend_parse_arg_array_ht(arg ## arg_num, &dest_ht, /* null_check */ false, /* or_object */ false, /* separate */ false)) { \ zend_wrong_parameter_type_error(arg_num, Z_EXPECTED_ARRAY, arg ## arg_num); \ goto flf_clean; \ - } -#define Z_FLF_PARAM_ARRAY_OR_NULL(arg_num, dest) \ - if (!zend_parse_arg_array(arg ## arg_num, &dest, /* null_check */ true, /* or_object */ false)) { \ + } \ + GC_TRY_ADDREF(dest_ht); +#define Z_FLF_PARAM_ARRAY_OR_NULL(arg_num, dest_ht) \ + if (!zend_parse_arg_array_ht(arg ## arg_num, &dest_ht, /* null_check */ true, /* or_object */ false, /* separate */ false)) { \ zend_wrong_parameter_type_error(arg_num, Z_EXPECTED_ARRAY_OR_NULL, arg ## arg_num); \ goto flf_clean; \ + } \ + if (dest_ht) { \ + GC_TRY_ADDREF(dest_ht); \ } #define Z_FLF_PARAM_ARRAY_HT_OR_STR(arg_num, dest_ht, dest_str, str_tmp) \ if (Z_TYPE_P(arg ## arg_num) == IS_STRING) { \ dest_ht = NULL; \ + ZVAL_COPY(&str_tmp, arg ## arg_num); \ + arg ## arg_num = &str_tmp; \ dest_str = Z_STR_P(arg ## arg_num); \ } else if (EXPECTED(Z_TYPE_P(arg ## arg_num) == IS_ARRAY)) { \ dest_ht = Z_ARRVAL_P(arg ## arg_num); \ dest_str = NULL; \ + GC_TRY_ADDREF(dest_ht); \ } else { \ dest_ht = NULL; \ ZVAL_COPY(&str_tmp, arg ## arg_num); \ @@ -99,6 +106,16 @@ if (UNEXPECTED(arg ## arg_num == &tmp)) { \ zval_ptr_dtor(arg ## arg_num); \ } +#define Z_FLF_PARAM_FREE_ARRAY(dest_ht) \ + if (dest_ht) { \ + GC_TRY_DTOR_NO_REF(dest_ht); \ + } +#define Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(arg_num, dest_ht, str_tmp) \ + if (dest_ht) { \ + GC_TRY_DTOR_NO_REF(dest_ht); \ + } else if (arg ## arg_num == &str_tmp) { \ + zval_ptr_dtor(arg ## arg_num); \ + } BEGIN_EXTERN_C() diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 4fde4ed5f9b9..22ccae21a421 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -2378,7 +2378,7 @@ PHP_FUNCTION(preg_replace) ZEND_FRAMELESS_FUNCTION(preg_replace, 3) { zend_string *regex_str, *replace_str, *subject_str; - HashTable *regex_ht, *replace_ht, *subject_ht; + HashTable *regex_ht = NULL, *replace_ht = NULL, *subject_ht = NULL; zval regex_tmp, replace_tmp, subject_tmp; Z_FLF_PARAM_ARRAY_HT_OR_STR(1, regex_ht, regex_str, regex_tmp); @@ -2393,9 +2393,9 @@ ZEND_FRAMELESS_FUNCTION(preg_replace, 3) /* limit */ -1, /* zcount */ NULL, /* is_filter */ false); flf_clean:; - Z_FLF_PARAM_FREE_STR(1, regex_tmp); - Z_FLF_PARAM_FREE_STR(2, replace_tmp); - Z_FLF_PARAM_FREE_STR(3, subject_tmp); + Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(1, regex_ht, regex_tmp); + Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(2, replace_ht, replace_tmp); + Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(3, subject_ht, subject_tmp); } /* {{{ Perform Perl-style regular expression replacement using replacement callback. */ diff --git a/ext/pcre/tests/gh23725.phpt b/ext/pcre/tests/gh23725.phpt new file mode 100644 index 000000000000..c0cd3f31c7e4 --- /dev/null +++ b/ext/pcre/tests/gh23725.phpt @@ -0,0 +1,129 @@ +--TEST-- +GH-23725 (Use-after-free when __toString() destroys a preg_replace() argument) +--FILE-- +getMessage(), "\n"; + } + var_dump($patterns); +} + +destroyedPatternArray(); +destroyedReplacementArray(); +destroyedSubjectArray(); +destroyedPatternString(); +appendedPatternArray(); +threw(); +?> +--EXPECT-- +pattern array: string(3) "zzz" +NULL +replacement array: string(3) "zyy" +NULL +subject array: array(2) { + [0]=> + string(3) "zbc" + [1]=> + string(3) "zbc" +} +NULL +pattern string: string(3) "zbc" +NULL +appended: string(3) "XXz" +count: 3 +Exception: boom +NULL diff --git a/ext/standard/array.c b/ext/standard/array.c index 0df44f8242ba..d90fe2c097f4 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1579,7 +1579,7 @@ PHP_FUNCTION(array_walk_recursive) * 0 = return boolean * 1 = return key */ -static zend_always_inline void _php_search_array(zval *return_value, zval *value, zval *array, bool strict, int behavior) /* {{{ */ +static zend_always_inline void _php_search_array(zval *return_value, zval *value, HashTable *array, bool strict, int behavior) /* {{{ */ { zval *entry; /* pointer to array entry */ zend_ulong num_idx; @@ -1587,7 +1587,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value if (strict) { if (Z_TYPE_P(value) == IS_LONG) { - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) { + ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) { ZVAL_DEREF(entry); if (Z_TYPE_P(entry) == IS_LONG && Z_LVAL_P(entry) == Z_LVAL_P(value)) { if (behavior == 0) { @@ -1602,7 +1602,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value } } ZEND_HASH_FOREACH_END(); } else { - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) { + ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) { ZVAL_DEREF(entry); if (fast_is_identical_function(value, entry)) { if (behavior == 0) { @@ -1619,7 +1619,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value } } else { if (Z_TYPE_P(value) == IS_LONG) { - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) { + ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) { if (fast_equal_check_long(value, entry)) { if (behavior == 0) { RETURN_TRUE; @@ -1633,7 +1633,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value } } ZEND_HASH_FOREACH_END(); } else if (Z_TYPE_P(value) == IS_STRING) { - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) { + ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) { if (fast_equal_check_string(value, entry)) { if (behavior == 0) { RETURN_TRUE; @@ -1647,7 +1647,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value } } ZEND_HASH_FOREACH_END(); } else { - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) { + ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) { if (fast_equal_check_function(value, entry)) { if (behavior == 0) { RETURN_TRUE; @@ -1673,13 +1673,13 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value */ static inline void php_search_array(INTERNAL_FUNCTION_PARAMETERS, int behavior) { - zval *value, /* value to check for */ - *array; /* array to check in */ + zval *value; /* value to check for */ + HashTable *array; /* array to check in */ bool strict = 0; /* strict comparison or not */ ZEND_PARSE_PARAMETERS_START(2, 3) Z_PARAM_ZVAL(value) - Z_PARAM_ARRAY(array) + Z_PARAM_ARRAY_HT(array) Z_PARAM_OPTIONAL Z_PARAM_BOOL(strict) ZEND_PARSE_PARAMETERS_END(); @@ -1696,7 +1696,8 @@ PHP_FUNCTION(in_array) ZEND_FRAMELESS_FUNCTION(in_array, 2) { - zval *value, *array; + zval *value; + HashTable *array = NULL; Z_FLF_PARAM_ZVAL(1, value); Z_FLF_PARAM_ARRAY(2, array); @@ -1704,11 +1705,13 @@ ZEND_FRAMELESS_FUNCTION(in_array, 2) _php_search_array(return_value, value, array, false, 0); flf_clean:; + Z_FLF_PARAM_FREE_ARRAY(array); } ZEND_FRAMELESS_FUNCTION(in_array, 3) { - zval *value, *array; + zval *value; + HashTable *array = NULL; bool strict; Z_FLF_PARAM_ZVAL(1, value); @@ -1718,6 +1721,7 @@ ZEND_FRAMELESS_FUNCTION(in_array, 3) _php_search_array(return_value, value, array, strict, 0); flf_clean:; + Z_FLF_PARAM_FREE_ARRAY(array); } /* {{{ Searches the array for a given value and returns the corresponding key if successful */ diff --git a/ext/standard/string.c b/ext/standard/string.c index 2c5990a6a8c8..a9abe82aa21b 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -1131,7 +1131,7 @@ ZEND_FRAMELESS_FUNCTION(implode, 2) { zval str_tmp; zend_string *str; - zval *pieces; + HashTable *pieces = NULL; Z_FLF_PARAM_STR(1, str, str_tmp); Z_FLF_PARAM_ARRAY_OR_NULL(2, pieces); @@ -1145,10 +1145,11 @@ ZEND_FRAMELESS_FUNCTION(implode, 2) goto flf_clean; } - php_implode(str, Z_ARR_P(pieces), return_value); + php_implode(str, pieces, return_value); flf_clean:; Z_FLF_PARAM_FREE_STR(1, str_tmp); + Z_FLF_PARAM_FREE_ARRAY(pieces); } #define STRTOK_TABLE(p) BG(strtok_table)[(unsigned char) *p] @@ -3558,7 +3559,7 @@ ZEND_FRAMELESS_FUNCTION(strtr, 2) { zval str_tmp; zend_string *str; - zval *from; + HashTable *from = NULL; Z_FLF_PARAM_STR(1, str, str_tmp); Z_FLF_PARAM_ARRAY(2, from); @@ -3568,10 +3569,11 @@ ZEND_FRAMELESS_FUNCTION(strtr, 2) goto flf_clean; } - php_strtr_array(return_value, str, Z_ARR_P(from)); + php_strtr_array(return_value, str, from); flf_clean: Z_FLF_PARAM_FREE_STR(1, str_tmp); + Z_FLF_PARAM_FREE_ARRAY(from); } ZEND_FRAMELESS_FUNCTION(strtr, 3) @@ -4660,7 +4662,7 @@ PHP_FUNCTION(str_replace) ZEND_FRAMELESS_FUNCTION(str_replace, 3) { zend_string *search_str, *replace_str, *subject_str; - HashTable *search_ht, *replace_ht, *subject_ht; + HashTable *search_ht = NULL, *replace_ht = NULL, *subject_ht = NULL; zval search_tmp, replace_tmp, subject_tmp; Z_FLF_PARAM_ARRAY_HT_OR_STR(1, search_ht, search_str, search_tmp); @@ -4670,9 +4672,9 @@ ZEND_FRAMELESS_FUNCTION(str_replace, 3) _php_str_replace_common(return_value, search_ht, search_str, replace_ht, replace_str, subject_ht, subject_str, /* zcount */ NULL, /* case_sensitivity */ true); flf_clean:; - Z_FLF_PARAM_FREE_STR(1, search_tmp); - Z_FLF_PARAM_FREE_STR(2, replace_tmp); - Z_FLF_PARAM_FREE_STR(3, subject_tmp); + Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(1, search_ht, search_tmp); + Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(2, replace_ht, replace_tmp); + Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(3, subject_ht, subject_tmp); } /* {{{ Replaces all occurrences of search in haystack with replace / case-insensitive */ diff --git a/ext/standard/tests/array/gh23722.phpt b/ext/standard/tests/array/gh23722.phpt new file mode 100644 index 000000000000..5371109980fa --- /dev/null +++ b/ext/standard/tests/array/gh23722.phpt @@ -0,0 +1,84 @@ +--TEST-- +GH-23722 (Use-after-free when __toString() destroys the in_array() haystack) +--FILE-- +getMessage(), "\n"; + } + var_dump($a); +} + +destroyed(); +destroyedWithStrictArg(); +appended(); +threw(); +?> +--EXPECT-- +destroyed: bool(false) +NULL +destroyed, with strict argument: bool(false) +NULL +appended: bool(false) +count: 6 +Exception: boom +NULL diff --git a/ext/standard/tests/strings/gh23725.phpt b/ext/standard/tests/strings/gh23725.phpt new file mode 100644 index 000000000000..c0844f50566f --- /dev/null +++ b/ext/standard/tests/strings/gh23725.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-23725 (Use-after-free when __toString() destroys a str_replace() string argument) +--FILE-- + +--EXPECT-- +search string, freed by replacement: string(2) "zz" +NULL +search string, freed by subject: string(2) "zz" +NULL