From 6c2ad99c9ef02494e205c880f9a88955c916e5bc Mon Sep 17 00:00:00 2001 From: lazerg Date: Tue, 11 Aug 2026 02:16:00 +0500 Subject: [PATCH 1/4] Fix GH-23204: use-after-free in implode() when __toString() destroys the array --- NEWS | 2 + ext/standard/string.c | 4 ++ ext/standard/tests/strings/gh23204.phpt | 49 +++++++++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 ext/standard/tests/strings/gh23204.phpt diff --git a/NEWS b/NEWS index 6447fa7bc881..75abece9efce 100644 --- a/NEWS +++ b/NEWS @@ -78,6 +78,8 @@ PHP NEWS nested arrays). (Lazizbek Ergashev) . Fixed bug GH-23115 (Stack overflow in compact() with deeply nested arrays). (Lazizbek Ergashev) + . Fixed bug GH-23204 (Use-after-free in implode() when __toString() destroys + the array being joined). (Lazizbek Ergashev) - Streams: . Fixed bug GH-15836 (Use-after-free when a user stream filter accesses diff --git a/ext/standard/string.c b/ext/standard/string.c index 0c7a7453eaab..1e43634539db 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -983,6 +983,9 @@ PHPAPI void php_implode(const zend_string *glue, HashTable *pieces, zval *return uint32_t flags = ZSTR_GET_COPYABLE_CONCAT_PROPERTIES(glue); + /* Converting an element may call __toString(), which can destroy pieces. */ + GC_TRY_ADDREF(pieces); + ZEND_HASH_FOREACH_VAL(pieces, tmp) { if (EXPECTED(Z_TYPE_P(tmp) == IS_STRING)) { ptr->str = Z_STR_P(tmp); @@ -1042,6 +1045,7 @@ PHPAPI void php_implode(const zend_string *glue, HashTable *pieces, zval *return } free_alloca(strings, use_heap); + GC_TRY_DTOR_NO_REF(pieces); RETURN_NEW_STR(str); } /* }}} */ diff --git a/ext/standard/tests/strings/gh23204.phpt b/ext/standard/tests/strings/gh23204.phpt new file mode 100644 index 000000000000..ead95997dd28 --- /dev/null +++ b/ext/standard/tests/strings/gh23204.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-23204 (Use-after-free in implode() when __toString() destroys the array) +--FILE-- +getMessage(), "\n"; +} +?> +--EXPECT-- +destroyed: X,2,3,4 +NULL +appended: X,2,3,4 +count: 5 +boom From 127f01680e31f4986556681e433dc5f73bfafd14 Mon Sep 17 00:00:00 2001 From: lazerg Date: Tue, 11 Aug 2026 02:59:34 +0500 Subject: [PATCH 2/4] Fix the same use-after-free in strtr() and str_replace() --- NEWS | 4 +- ext/standard/string.c | 32 ++++++++++++- ext/standard/tests/strings/gh23204.phpt | 60 ++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 75abece9efce..6fb6fe79a82d 100644 --- a/NEWS +++ b/NEWS @@ -78,8 +78,8 @@ PHP NEWS nested arrays). (Lazizbek Ergashev) . Fixed bug GH-23115 (Stack overflow in compact() with deeply nested arrays). (Lazizbek Ergashev) - . Fixed bug GH-23204 (Use-after-free in implode() when __toString() destroys - the array being joined). (Lazizbek Ergashev) + . Fixed bug GH-23204 (Use-after-free in implode(), strtr() and str_replace() + when __toString() destroys an array argument). (Lazizbek Ergashev) - Streams: . Fixed bug GH-15836 (Use-after-free when a user stream filter accesses diff --git a/ext/standard/string.c b/ext/standard/string.c index 1e43634539db..34444d80d185 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -3396,7 +3396,12 @@ static void php_strtr_array(zval *return_value, zend_string *str, HashTable *fro { if (zend_hash_num_elements(from_ht) < 1) { RETURN_STR_COPY(str); - } else if (zend_hash_num_elements(from_ht) == 1) { + } + + /* Converting a replacement may call __toString(), which can destroy from_ht. */ + GC_TRY_ADDREF(from_ht); + + if (zend_hash_num_elements(from_ht) == 1) { zend_long num_key; zend_string *str_key, *tmp_str, *replace, *tmp_replace; zval *entry; @@ -3425,11 +3430,13 @@ static void php_strtr_array(zval *return_value, zend_string *str, HashTable *fro } zend_tmp_string_release(tmp_str); zend_tmp_string_release(tmp_replace); - return; + break; } ZEND_HASH_FOREACH_END(); } else { php_strtr_array_ex(return_value, str, from_ht); } + + GC_TRY_DTOR_NO_REF(from_ht); } /* {{{ Translates characters in str using given translation tables */ @@ -4489,6 +4496,17 @@ static void _php_str_replace_common( RETURN_THROWS(); } + /* Converting an element may call __toString(), which can destroy the arrays. */ + if (search_ht) { + GC_TRY_ADDREF(search_ht); + } + if (replace_ht) { + GC_TRY_ADDREF(replace_ht); + } + if (subject_ht) { + GC_TRY_ADDREF(subject_ht); + } + /* if subject is an array */ if (subject_ht) { array_init(return_value); @@ -4515,6 +4533,16 @@ static void _php_str_replace_common( if (zcount) { ZEND_TRY_ASSIGN_REF_LONG(zcount, count); } + + if (search_ht) { + GC_TRY_DTOR_NO_REF(search_ht); + } + if (replace_ht) { + GC_TRY_DTOR_NO_REF(replace_ht); + } + if (subject_ht) { + GC_TRY_DTOR_NO_REF(subject_ht); + } } /* {{{ php_str_replace_common */ diff --git a/ext/standard/tests/strings/gh23204.phpt b/ext/standard/tests/strings/gh23204.phpt index ead95997dd28..c4ab1ac546bc 100644 --- a/ext/standard/tests/strings/gh23204.phpt +++ b/ext/standard/tests/strings/gh23204.phpt @@ -1,5 +1,5 @@ --TEST-- -GH-23204 (Use-after-free in implode() when __toString() destroys the array) +GH-23204 (Use-after-free when __toString() destroys the array being read) --FILE-- getMessage(), "\n"; } + +class UnsetPats implements Stringable { + public function __toString(): string { + global $d; + $d = null; + return "X"; + } +} + +$d = ["aa" => new UnsetPats, "bb" => "2", "cc" => "3", "dd" => "4"]; +echo "strtr: ", strtr("aabbccdd", $d), "\n"; + +$e = ["aa" => new UnsetPats]; +$d = &$e; +echo "strtr single: ", strtr("aabb", $e), "\n"; + +class UnsetSearch implements Stringable { + public function __toString(): string { + global $f; + $f = null; + return "a"; + } +} + +$f = [new UnsetSearch, "b", "c", "d"]; +echo "str_replace search: ", str_replace($f, "z", "abcd"), "\n"; + +class UnsetReplace implements Stringable { + public function __toString(): string { + global $g; + $g = null; + return "z"; + } +} + +$g = [new UnsetReplace, "y", "y", "y"]; +echo "str_replace replace: ", str_replace(["a", "b", "c", "d"], $g, "abcd"), "\n"; + +class UnsetSubject implements Stringable { + public function __toString(): string { + global $h; + $h = null; + return "abcd"; + } +} + +$h = [new UnsetSubject, "abcd"]; +var_dump(str_replace("a", "z", $h)); ?> --EXPECT-- destroyed: X,2,3,4 @@ -47,3 +95,13 @@ NULL appended: X,2,3,4 count: 5 boom +strtr: X234 +strtr single: Xbb +str_replace search: zzzz +str_replace replace: zyyy +array(2) { + [0]=> + string(4) "zbcd" + [1]=> + string(4) "zbcd" +} From b9443b8534c25adb016a78086266de6445f9447b Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Tue, 11 Aug 2026 05:18:32 +0500 Subject: [PATCH 3/4] Credit the reporter in the GH-23204 test --- ext/standard/tests/strings/gh23204.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/standard/tests/strings/gh23204.phpt b/ext/standard/tests/strings/gh23204.phpt index c4ab1ac546bc..0fcbb29de904 100644 --- a/ext/standard/tests/strings/gh23204.phpt +++ b/ext/standard/tests/strings/gh23204.phpt @@ -1,5 +1,7 @@ --TEST-- GH-23204 (Use-after-free when __toString() destroys the array being read) +--CREDITS-- +iluuu1994 --FILE-- Date: Tue, 11 Aug 2026 13:47:39 +0500 Subject: [PATCH 4/4] Credit e1abrador and assert the exception class in the GH-23204 test --- ext/standard/tests/strings/gh23204.phpt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/standard/tests/strings/gh23204.phpt b/ext/standard/tests/strings/gh23204.phpt index 0fcbb29de904..e2ae20592c5a 100644 --- a/ext/standard/tests/strings/gh23204.phpt +++ b/ext/standard/tests/strings/gh23204.phpt @@ -1,7 +1,7 @@ --TEST-- GH-23204 (Use-after-free when __toString() destroys the array being read) --CREDITS-- -iluuu1994 +e1abrador --FILE-- getMessage(), "\n"; + echo $e::class, ': ', $e->getMessage(), "\n"; } class UnsetPats implements Stringable { @@ -96,7 +96,7 @@ destroyed: X,2,3,4 NULL appended: X,2,3,4 count: 5 -boom +Exception: boom strtr: X234 strtr single: Xbb str_replace search: zzzz