Skip to content

Commit 4e514ed

Browse files
iliaaliliaal
committed
Fix GH-23725: use-after-free when __toString() frees a frameless argument
Frameless calls pass the caller's operand zvals straight to the handler without taking a reference, so an argument freed by a __toString() that the handler itself triggers leaves it reading freed memory. Take a reference on array and array-or-string arguments in the Z_FLF_PARAM_ARRAY* macros and release it at flf_clean, so a destructive write from userland separates the array instead. The handle is kept in a local because the operand may be a reference slot that userland overwrites. This covers in_array(), preg_replace(), str_replace(), strtr() and implode(); the per-function guards from 8ce7f7f stay, since implode/1 parses its argument without the macros and php_implode() is public API. Z_FLF_PARAM_STR still reads its string directly on the fast path, so GH-21639 stays open. Fixes GH-23725
1 parent bd9529d commit 4e514ed

8 files changed

Lines changed: 316 additions & 28 deletions

File tree

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ PHP NEWS
33
?? ??? ????, PHP 8.6.0RC1
44

55
- Core:
6+
. Fixed bug GH-23725 (use-after-free when __toString() destroys an array or
7+
string argument of a frameless in_array(), preg_replace() or
8+
str_replace() call). (Ilia Alshanetsky)
69
. Fixed incorrect internal pointer and foreach iterator positions when
710
compacting arrays with holes. (Weilin Du)
811
. Fix handling of references to typed properties during unserialization

‎Zend/zend_frameless_function.h‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,23 +43,30 @@
4343

4444
#define Z_FLF_PARAM_ZVAL(arg_num, dest) \
4545
dest = arg ## arg_num;
46-
#define Z_FLF_PARAM_ARRAY(arg_num, dest) \
47-
if (!zend_parse_arg_array(arg ## arg_num, &dest, /* null_check */ false, /* or_object */ false)) { \
46+
#define Z_FLF_PARAM_ARRAY(arg_num, dest_ht) \
47+
if (!zend_parse_arg_array_ht(arg ## arg_num, &dest_ht, /* null_check */ false, /* or_object */ false, /* separate */ false)) { \
4848
zend_wrong_parameter_type_error(arg_num, Z_EXPECTED_ARRAY, arg ## arg_num); \
4949
goto flf_clean; \
50-
}
51-
#define Z_FLF_PARAM_ARRAY_OR_NULL(arg_num, dest) \
52-
if (!zend_parse_arg_array(arg ## arg_num, &dest, /* null_check */ true, /* or_object */ false)) { \
50+
} \
51+
GC_TRY_ADDREF(dest_ht);
52+
#define Z_FLF_PARAM_ARRAY_OR_NULL(arg_num, dest_ht) \
53+
if (!zend_parse_arg_array_ht(arg ## arg_num, &dest_ht, /* null_check */ true, /* or_object */ false, /* separate */ false)) { \
5354
zend_wrong_parameter_type_error(arg_num, Z_EXPECTED_ARRAY_OR_NULL, arg ## arg_num); \
5455
goto flf_clean; \
56+
} \
57+
if (dest_ht) { \
58+
GC_TRY_ADDREF(dest_ht); \
5559
}
5660
#define Z_FLF_PARAM_ARRAY_HT_OR_STR(arg_num, dest_ht, dest_str, str_tmp) \
5761
if (Z_TYPE_P(arg ## arg_num) == IS_STRING) { \
5862
dest_ht = NULL; \
63+
ZVAL_COPY(&str_tmp, arg ## arg_num); \
64+
arg ## arg_num = &str_tmp; \
5965
dest_str = Z_STR_P(arg ## arg_num); \
6066
} else if (EXPECTED(Z_TYPE_P(arg ## arg_num) == IS_ARRAY)) { \
6167
dest_ht = Z_ARRVAL_P(arg ## arg_num); \
6268
dest_str = NULL; \
69+
GC_TRY_ADDREF(dest_ht); \
6370
} else { \
6471
dest_ht = NULL; \
6572
ZVAL_COPY(&str_tmp, arg ## arg_num); \
@@ -99,6 +106,16 @@
99106
if (UNEXPECTED(arg ## arg_num == &tmp)) { \
100107
zval_ptr_dtor(arg ## arg_num); \
101108
}
109+
#define Z_FLF_PARAM_FREE_ARRAY(dest_ht) \
110+
if (dest_ht) { \
111+
GC_TRY_DTOR_NO_REF(dest_ht); \
112+
}
113+
#define Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(arg_num, dest_ht, str_tmp) \
114+
if (dest_ht) { \
115+
GC_TRY_DTOR_NO_REF(dest_ht); \
116+
} else if (arg ## arg_num == &str_tmp) { \
117+
zval_ptr_dtor(arg ## arg_num); \
118+
}
102119

103120
BEGIN_EXTERN_C()
104121

‎ext/pcre/php_pcre.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2378,7 +2378,7 @@ PHP_FUNCTION(preg_replace)
23782378
ZEND_FRAMELESS_FUNCTION(preg_replace, 3)
23792379
{
23802380
zend_string *regex_str, *replace_str, *subject_str;
2381-
HashTable *regex_ht, *replace_ht, *subject_ht;
2381+
HashTable *regex_ht = NULL, *replace_ht = NULL, *subject_ht = NULL;
23822382
zval regex_tmp, replace_tmp, subject_tmp;
23832383

23842384
Z_FLF_PARAM_ARRAY_HT_OR_STR(1, regex_ht, regex_str, regex_tmp);
@@ -2393,9 +2393,9 @@ ZEND_FRAMELESS_FUNCTION(preg_replace, 3)
23932393
/* limit */ -1, /* zcount */ NULL, /* is_filter */ false);
23942394

23952395
flf_clean:;
2396-
Z_FLF_PARAM_FREE_STR(1, regex_tmp);
2397-
Z_FLF_PARAM_FREE_STR(2, replace_tmp);
2398-
Z_FLF_PARAM_FREE_STR(3, subject_tmp);
2396+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(1, regex_ht, regex_tmp);
2397+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(2, replace_ht, replace_tmp);
2398+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(3, subject_ht, subject_tmp);
23992399
}
24002400

24012401
/* {{{ Perform Perl-style regular expression replacement using replacement callback. */

‎ext/pcre/tests/gh23725.phpt‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
--TEST--
2+
GH-23725 (Use-after-free when __toString() destroys a preg_replace() argument)
3+
--FILE--
4+
<?php
5+
class UnsetPatterns implements Stringable {
6+
public function __toString(): string {
7+
global $patterns;
8+
$patterns = null;
9+
return "/a/";
10+
}
11+
}
12+
13+
class UnsetReplacements implements Stringable {
14+
public function __toString(): string {
15+
global $replacements;
16+
$replacements = null;
17+
return "z";
18+
}
19+
}
20+
21+
class UnsetSubjects implements Stringable {
22+
public function __toString(): string {
23+
global $subjects;
24+
$subjects = null;
25+
return "abc";
26+
}
27+
}
28+
29+
class UnsetPatternString implements Stringable {
30+
public function __toString(): string {
31+
global $pattern;
32+
$pattern = null;
33+
return "z";
34+
}
35+
}
36+
37+
class AppendPatterns implements Stringable {
38+
public function __toString(): string {
39+
global $patterns;
40+
$patterns[] = "/z/";
41+
return "/a/";
42+
}
43+
}
44+
45+
class Boom implements Stringable {
46+
public function __toString(): string {
47+
global $patterns;
48+
$patterns = null;
49+
throw new Exception("boom");
50+
}
51+
}
52+
53+
function destroyedPatternArray(): void {
54+
global $patterns;
55+
$patterns = [new UnsetPatterns, "/b/", "/c/"];
56+
echo "pattern array: ";
57+
var_dump(preg_replace($patterns, "z", "abc"));
58+
var_dump($patterns);
59+
}
60+
61+
function destroyedReplacementArray(): void {
62+
global $replacements;
63+
$replacements = [new UnsetReplacements, "y", "y"];
64+
echo "replacement array: ";
65+
var_dump(preg_replace(["/a/", "/b/", "/c/"], $replacements, "abc"));
66+
var_dump($replacements);
67+
}
68+
69+
function destroyedSubjectArray(): void {
70+
global $subjects;
71+
$subjects = [new UnsetSubjects, "abc"];
72+
echo "subject array: ";
73+
var_dump(preg_replace("/a/", "z", $subjects));
74+
var_dump($subjects);
75+
}
76+
77+
function destroyedPatternString(): void {
78+
global $pattern;
79+
$sep = "/";
80+
$pattern = $sep . "a" . $sep;
81+
echo "pattern string: ";
82+
var_dump(preg_replace($pattern, new UnsetPatternString, "abc"));
83+
var_dump($pattern);
84+
}
85+
86+
function appendedPatternArray(): void {
87+
global $patterns;
88+
$patterns = [new AppendPatterns, "/b/"];
89+
echo "appended: ";
90+
var_dump(preg_replace($patterns, "X", "abz"));
91+
echo "count: ", count($patterns), "\n";
92+
}
93+
94+
function threw(): void {
95+
global $patterns;
96+
$patterns = [new Boom, "/b/"];
97+
try {
98+
var_dump(preg_replace($patterns, "X", "ab"));
99+
} catch (Exception $e) {
100+
echo $e::class, ': ', $e->getMessage(), "\n";
101+
}
102+
var_dump($patterns);
103+
}
104+
105+
destroyedPatternArray();
106+
destroyedReplacementArray();
107+
destroyedSubjectArray();
108+
destroyedPatternString();
109+
appendedPatternArray();
110+
threw();
111+
?>
112+
--EXPECT--
113+
pattern array: string(3) "zzz"
114+
NULL
115+
replacement array: string(3) "zyy"
116+
NULL
117+
subject array: array(2) {
118+
[0]=>
119+
string(3) "zbc"
120+
[1]=>
121+
string(3) "zbc"
122+
}
123+
NULL
124+
pattern string: string(3) "zbc"
125+
NULL
126+
appended: string(3) "XXz"
127+
count: 3
128+
Exception: boom
129+
NULL

‎ext/standard/array.c‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,15 +1579,15 @@ PHP_FUNCTION(array_walk_recursive)
15791579
* 0 = return boolean
15801580
* 1 = return key
15811581
*/
1582-
static zend_always_inline void _php_search_array(zval *return_value, zval *value, zval *array, bool strict, int behavior) /* {{{ */
1582+
static zend_always_inline void _php_search_array(zval *return_value, zval *value, HashTable *array, bool strict, int behavior) /* {{{ */
15831583
{
15841584
zval *entry; /* pointer to array entry */
15851585
zend_ulong num_idx;
15861586
zend_string *str_idx;
15871587

15881588
if (strict) {
15891589
if (Z_TYPE_P(value) == IS_LONG) {
1590-
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) {
1590+
ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) {
15911591
ZVAL_DEREF(entry);
15921592
if (Z_TYPE_P(entry) == IS_LONG && Z_LVAL_P(entry) == Z_LVAL_P(value)) {
15931593
if (behavior == 0) {
@@ -1602,7 +1602,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value
16021602
}
16031603
} ZEND_HASH_FOREACH_END();
16041604
} else {
1605-
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) {
1605+
ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) {
16061606
ZVAL_DEREF(entry);
16071607
if (fast_is_identical_function(value, entry)) {
16081608
if (behavior == 0) {
@@ -1619,7 +1619,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value
16191619
}
16201620
} else {
16211621
if (Z_TYPE_P(value) == IS_LONG) {
1622-
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) {
1622+
ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) {
16231623
if (fast_equal_check_long(value, entry)) {
16241624
if (behavior == 0) {
16251625
RETURN_TRUE;
@@ -1633,7 +1633,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value
16331633
}
16341634
} ZEND_HASH_FOREACH_END();
16351635
} else if (Z_TYPE_P(value) == IS_STRING) {
1636-
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) {
1636+
ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) {
16371637
if (fast_equal_check_string(value, entry)) {
16381638
if (behavior == 0) {
16391639
RETURN_TRUE;
@@ -1647,7 +1647,7 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value
16471647
}
16481648
} ZEND_HASH_FOREACH_END();
16491649
} else {
1650-
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_idx, str_idx, entry) {
1650+
ZEND_HASH_FOREACH_KEY_VAL(array, num_idx, str_idx, entry) {
16511651
if (fast_equal_check_function(value, entry)) {
16521652
if (behavior == 0) {
16531653
RETURN_TRUE;
@@ -1673,13 +1673,13 @@ static zend_always_inline void _php_search_array(zval *return_value, zval *value
16731673
*/
16741674
static inline void php_search_array(INTERNAL_FUNCTION_PARAMETERS, int behavior)
16751675
{
1676-
zval *value, /* value to check for */
1677-
*array; /* array to check in */
1676+
zval *value; /* value to check for */
1677+
HashTable *array; /* array to check in */
16781678
bool strict = 0; /* strict comparison or not */
16791679

16801680
ZEND_PARSE_PARAMETERS_START(2, 3)
16811681
Z_PARAM_ZVAL(value)
1682-
Z_PARAM_ARRAY(array)
1682+
Z_PARAM_ARRAY_HT(array)
16831683
Z_PARAM_OPTIONAL
16841684
Z_PARAM_BOOL(strict)
16851685
ZEND_PARSE_PARAMETERS_END();
@@ -1696,19 +1696,22 @@ PHP_FUNCTION(in_array)
16961696

16971697
ZEND_FRAMELESS_FUNCTION(in_array, 2)
16981698
{
1699-
zval *value, *array;
1699+
zval *value;
1700+
HashTable *array = NULL;
17001701

17011702
Z_FLF_PARAM_ZVAL(1, value);
17021703
Z_FLF_PARAM_ARRAY(2, array);
17031704

17041705
_php_search_array(return_value, value, array, false, 0);
17051706

17061707
flf_clean:;
1708+
Z_FLF_PARAM_FREE_ARRAY(array);
17071709
}
17081710

17091711
ZEND_FRAMELESS_FUNCTION(in_array, 3)
17101712
{
1711-
zval *value, *array;
1713+
zval *value;
1714+
HashTable *array = NULL;
17121715
bool strict;
17131716

17141717
Z_FLF_PARAM_ZVAL(1, value);
@@ -1718,6 +1721,7 @@ ZEND_FRAMELESS_FUNCTION(in_array, 3)
17181721
_php_search_array(return_value, value, array, strict, 0);
17191722

17201723
flf_clean:;
1724+
Z_FLF_PARAM_FREE_ARRAY(array);
17211725
}
17221726

17231727
/* {{{ Searches the array for a given value and returns the corresponding key if successful */

‎ext/standard/string.c‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,7 +1131,7 @@ ZEND_FRAMELESS_FUNCTION(implode, 2)
11311131
{
11321132
zval str_tmp;
11331133
zend_string *str;
1134-
zval *pieces;
1134+
HashTable *pieces = NULL;
11351135

11361136
Z_FLF_PARAM_STR(1, str, str_tmp);
11371137
Z_FLF_PARAM_ARRAY_OR_NULL(2, pieces);
@@ -1145,10 +1145,11 @@ ZEND_FRAMELESS_FUNCTION(implode, 2)
11451145
goto flf_clean;
11461146
}
11471147

1148-
php_implode(str, Z_ARR_P(pieces), return_value);
1148+
php_implode(str, pieces, return_value);
11491149

11501150
flf_clean:;
11511151
Z_FLF_PARAM_FREE_STR(1, str_tmp);
1152+
Z_FLF_PARAM_FREE_ARRAY(pieces);
11521153
}
11531154

11541155
#define STRTOK_TABLE(p) BG(strtok_table)[(unsigned char) *p]
@@ -3558,7 +3559,7 @@ ZEND_FRAMELESS_FUNCTION(strtr, 2)
35583559
{
35593560
zval str_tmp;
35603561
zend_string *str;
3561-
zval *from;
3562+
HashTable *from = NULL;
35623563

35633564
Z_FLF_PARAM_STR(1, str, str_tmp);
35643565
Z_FLF_PARAM_ARRAY(2, from);
@@ -3568,10 +3569,11 @@ ZEND_FRAMELESS_FUNCTION(strtr, 2)
35683569
goto flf_clean;
35693570
}
35703571

3571-
php_strtr_array(return_value, str, Z_ARR_P(from));
3572+
php_strtr_array(return_value, str, from);
35723573

35733574
flf_clean:
35743575
Z_FLF_PARAM_FREE_STR(1, str_tmp);
3576+
Z_FLF_PARAM_FREE_ARRAY(from);
35753577
}
35763578

35773579
ZEND_FRAMELESS_FUNCTION(strtr, 3)
@@ -4660,7 +4662,7 @@ PHP_FUNCTION(str_replace)
46604662
ZEND_FRAMELESS_FUNCTION(str_replace, 3)
46614663
{
46624664
zend_string *search_str, *replace_str, *subject_str;
4663-
HashTable *search_ht, *replace_ht, *subject_ht;
4665+
HashTable *search_ht = NULL, *replace_ht = NULL, *subject_ht = NULL;
46644666
zval search_tmp, replace_tmp, subject_tmp;
46654667

46664668
Z_FLF_PARAM_ARRAY_HT_OR_STR(1, search_ht, search_str, search_tmp);
@@ -4670,9 +4672,9 @@ ZEND_FRAMELESS_FUNCTION(str_replace, 3)
46704672
_php_str_replace_common(return_value, search_ht, search_str, replace_ht, replace_str, subject_ht, subject_str, /* zcount */ NULL, /* case_sensitivity */ true);
46714673

46724674
flf_clean:;
4673-
Z_FLF_PARAM_FREE_STR(1, search_tmp);
4674-
Z_FLF_PARAM_FREE_STR(2, replace_tmp);
4675-
Z_FLF_PARAM_FREE_STR(3, subject_tmp);
4675+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(1, search_ht, search_tmp);
4676+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(2, replace_ht, replace_tmp);
4677+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(3, subject_ht, subject_tmp);
46764678
}
46774679

46784680
/* {{{ Replaces all occurrences of search in haystack with replace / case-insensitive */

0 commit comments

Comments
 (0)