Skip to content

Commit cf6284b

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 user code that the handler triggers leaves it reading freed memory. Take a reference on array arguments in the Z_FLF_PARAM_ARRAY* macros and in the one-argument implode(), which makes the matching guards from 8ce7f7f redundant. String arguments are pinned only when user code can still run after they are read: a later argument needs conversion, strtr() walks a replacement array, implode() converts an element, or property_exists() may autoload. str_replace() and preg_replace() skip the pins when all three arguments are already strings. Fixes GH-23725
1 parent bd9529d commit cf6284b

11 files changed

Lines changed: 447 additions & 79 deletions

File tree

‎NEWS‎

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

55
- Core:
6+
. Fixed bug GH-23725 (use-after-free when __toString() destroys an argument
7+
of a frameless function call). (Ilia Alshanetsky)
68
. Fixed incorrect internal pointer and foreach iterator positions when
79
compacting arrays with holes. (Weilin Du)
810
. Fix handling of references to typed properties during unserialization

‎Zend/zend_builtin_functions.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1089,7 +1089,7 @@ ZEND_FRAMELESS_FUNCTION(property_exists, 2)
10891089
zend_string *property;
10901090

10911091
Z_FLF_PARAM_ZVAL(1, object);
1092-
Z_FLF_PARAM_STR(2, property, property_tmp);
1092+
Z_FLF_PARAM_STR_EX(2, property, property_tmp, Z_TYPE_P(arg1) == IS_STRING);
10931093

10941094
_property_exists(return_value, object, property);
10951095

‎Zend/zend_frameless_function.h‎

Lines changed: 37 additions & 6 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); \
@@ -85,7 +92,13 @@
8592
goto flf_clean; \
8693
}
8794
#define Z_FLF_PARAM_STR(arg_num, dest, tmp) \
95+
Z_FLF_PARAM_STR_EX(arg_num, dest, tmp, false)
96+
#define Z_FLF_PARAM_STR_EX(arg_num, dest, tmp, pin) \
8897
if (Z_TYPE_P(arg ## arg_num) == IS_STRING) { \
98+
if (UNEXPECTED(pin)) { \
99+
ZVAL_COPY(&tmp, arg ## arg_num); \
100+
arg ## arg_num = &tmp; \
101+
} \
89102
dest = Z_STR_P(arg ## arg_num); \
90103
} else { \
91104
ZVAL_COPY(&tmp, arg ## arg_num); \
@@ -97,7 +110,25 @@
97110
}
98111
#define Z_FLF_PARAM_FREE_STR(arg_num, tmp) \
99112
if (UNEXPECTED(arg ## arg_num == &tmp)) { \
100-
zval_ptr_dtor(arg ## arg_num); \
113+
if (EXPECTED(Z_TYPE(tmp) == IS_STRING)) { \
114+
zend_string_release_ex(Z_STR(tmp), false); \
115+
} else { \
116+
zval_ptr_dtor(&tmp); \
117+
} \
118+
}
119+
#define Z_FLF_PARAM_FREE_ARRAY(dest_ht) \
120+
if (dest_ht) { \
121+
GC_TRY_DTOR_NO_REF(dest_ht); \
122+
}
123+
#define Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(arg_num, dest_ht, str_tmp) \
124+
if (dest_ht) { \
125+
GC_TRY_DTOR_NO_REF(dest_ht); \
126+
} else if (arg ## arg_num == &str_tmp) { \
127+
if (EXPECTED(Z_TYPE(str_tmp) == IS_STRING)) { \
128+
zend_string_release_ex(Z_STR(str_tmp), false); \
129+
} else { \
130+
zval_ptr_dtor(&str_tmp); \
131+
} \
101132
}
102133

103134
BEGIN_EXTERN_C()

‎ext/pcre/php_pcre.c‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,7 +1488,7 @@ ZEND_FRAMELESS_FUNCTION(preg_match, 2)
14881488
zval regex_tmp, subject_tmp;
14891489
zend_string *regex, *subject;
14901490

1491-
Z_FLF_PARAM_STR(1, regex, regex_tmp);
1491+
Z_FLF_PARAM_STR_EX(1, regex, regex_tmp, Z_TYPE_P(arg2) != IS_STRING);
14921492
Z_FLF_PARAM_STR(2, subject, subject_tmp);
14931493

14941494
/* Compile regex or get it from cache. */
@@ -2378,9 +2378,19 @@ 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

2384+
if (EXPECTED(Z_TYPE_P(arg1) == IS_STRING && Z_TYPE_P(arg2) == IS_STRING && Z_TYPE_P(arg3) == IS_STRING)) {
2385+
_preg_replace_common(
2386+
return_value,
2387+
NULL, Z_STR_P(arg1),
2388+
NULL, Z_STR_P(arg2),
2389+
NULL, Z_STR_P(arg3),
2390+
-1, NULL, false);
2391+
return;
2392+
}
2393+
23842394
Z_FLF_PARAM_ARRAY_HT_OR_STR(1, regex_ht, regex_str, regex_tmp);
23852395
Z_FLF_PARAM_ARRAY_HT_OR_STR(2, replace_ht, replace_str, replace_tmp);
23862396
Z_FLF_PARAM_ARRAY_HT_OR_STR(3, subject_ht, subject_str, subject_tmp);
@@ -2393,9 +2403,9 @@ ZEND_FRAMELESS_FUNCTION(preg_replace, 3)
23932403
/* limit */ -1, /* zcount */ NULL, /* is_filter */ false);
23942404

23952405
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);
2406+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(1, regex_ht, regex_tmp);
2407+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(2, replace_ht, replace_tmp);
2408+
Z_FLF_PARAM_FREE_ARRAY_HT_OR_STR(3, subject_ht, subject_tmp);
23992409
}
24002410

24012411
/* {{{ 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 */

0 commit comments

Comments
 (0)