From 19ac30f99bd0cabc4445f40c840c003ee39f7a42 Mon Sep 17 00:00:00 2001 From: ColumbusLabs <287001685+ColumbusLabs@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:45:56 +0800 Subject: [PATCH 1/3] Fix GH-23094: Use byte offsets in NumberFormatter parsing (#23318) PHP exposes NumberFormatter parsing offsets as UTF-8 byte offsets, while ICU expects UTF-16 code-unit positions. Convert the input offset before parsing and the returned offset afterward for both parse() and parseCurrency(). Reject offsets that split a UTF-8 sequence, set the formatter error state, and leave the referenced offset unchanged. Closes #23318 --- NEWS | 2 ++ ext/intl/formatter/formatter_parse.c | 46 +++++++++++++++++++++++++++ ext/intl/tests/gh23094.phpt | 47 ++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 ext/intl/tests/gh23094.phpt diff --git a/NEWS b/NEWS index 8f462c66a26b..53bb8f7b1fb2 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ PHP NEWS - Intl: . Fixed a double-free when IntlGregorianCalendar construction fails after the ICU constructor adopts the TimeZone. (iliaal) + . Fixed bug GH-23094 (NumberFormatter parsing offsets use UTF-16 positions + for UTF-8 strings). (ColumbusLabs) - Opcache: . Fixed opcache.protect_memory race under ZTS. (realFlowControl) diff --git a/ext/intl/formatter/formatter_parse.c b/ext/intl/formatter/formatter_parse.c index 993990065040..2c5ac3222f3d 100644 --- a/ext/intl/formatter/formatter_parse.c +++ b/ext/intl/formatter/formatter_parse.c @@ -27,6 +27,42 @@ #define ICU_LOCALE_BUG 1 +static bool numfmt_utf8_offset_to_utf16(const char *str, size_t str_len, int32_t *position, UErrorCode *status) +{ + int32_t utf16_position; + + if (*position < 0 || (size_t) *position > str_len) { + return true; + } + + *status = U_ZERO_ERROR; + u_strFromUTF8(NULL, 0, &utf16_position, str, *position, status); + if (*status != U_BUFFER_OVERFLOW_ERROR && U_FAILURE(*status)) { + return false; + } + *status = U_ZERO_ERROR; + + *position = utf16_position; + return true; +} + +static int32_t numfmt_utf16_offset_to_utf8(const UChar *str, int32_t str_len, int32_t position) +{ + int32_t utf8_position; + UErrorCode status = U_ZERO_ERROR; + + if (position < 0 || position > str_len) { + return position; + } + + u_strToUTF8(NULL, 0, &utf8_position, str, position, &status); + if (status != U_BUFFER_OVERFLOW_ERROR && U_FAILURE(status)) { + return position; + } + + return utf8_position; +} + /* {{{ Parse a number. */ PHP_FUNCTION( numfmt_parse ) { @@ -61,6 +97,10 @@ PHP_FUNCTION( numfmt_parse ) /* Convert given string to UTF-16. */ intl_convert_utf8_to_utf16(&sstr, &sstr_len, str, str_len, &INTL_DATA_ERROR_CODE(nfo)); INTL_METHOD_CHECK_STATUS( nfo, "String conversion to UTF-16 failed" ); + if (zposition && !numfmt_utf8_offset_to_utf16(str, str_len, &position, &INTL_DATA_ERROR_CODE(nfo))) { + efree(sstr); + INTL_METHOD_CHECK_STATUS(nfo, "Invalid UTF-8 offset"); + } #if ICU_LOCALE_BUG && defined(LC_NUMERIC) /* need to copy here since setlocale may change it later */ @@ -101,6 +141,7 @@ PHP_FUNCTION( numfmt_parse ) } if (zposition) { + position = numfmt_utf16_offset_to_utf8(sstr, sstr_len, position); ZEND_TRY_ASSIGN_REF_LONG(zposition, position); } @@ -150,11 +191,16 @@ PHP_FUNCTION( numfmt_parse_currency ) if(zposition) { position = (int32_t) zval_get_long(zposition); + if (!numfmt_utf8_offset_to_utf16(str, str_len, &position, &INTL_DATA_ERROR_CODE(nfo))) { + efree(sstr); + INTL_METHOD_CHECK_STATUS(nfo, "Invalid UTF-8 offset"); + } position_p = &position; } number = unum_parseDoubleCurrency(FORMATTER_OBJECT(nfo), sstr, sstr_len, position_p, currency, &INTL_DATA_ERROR_CODE(nfo)); if(zposition) { + position = numfmt_utf16_offset_to_utf8(sstr, sstr_len, position); ZEND_TRY_ASSIGN_REF_LONG(zposition, position); } if (sstr) { diff --git a/ext/intl/tests/gh23094.phpt b/ext/intl/tests/gh23094.phpt new file mode 100644 index 000000000000..9ace16fa481f --- /dev/null +++ b/ext/intl/tests/gh23094.phpt @@ -0,0 +1,47 @@ +--TEST-- +GH-23094 NumberFormatter parse offsets use UTF-8 byte positions +--EXTENSIONS-- +intl +--FILE-- +parse($prefix . '123', NumberFormatter::TYPE_INT32, $offset)); +var_dump($offset); + +$offset = 1; +var_dump($formatter->parse("\u{00E9}123", NumberFormatter::TYPE_INT32, $offset)); +var_dump($offset); +var_dump(intl_is_failure($formatter->getErrorCode())); + +$formatter = new NumberFormatter('en_US', NumberFormatter::CURRENCY); +$offset = strlen($prefix); +$currency = null; +var_dump($formatter->parseCurrency($prefix . '$123.45', $currency, $offset)); +var_dump($currency); +var_dump($offset); + +$offset = 1; +$currency = null; +var_dump($formatter->parseCurrency("\u{00E9}$123.45", $currency, $offset)); +var_dump($currency); +var_dump($offset); +var_dump(intl_is_failure($formatter->getErrorCode())); + +?> +--EXPECT-- +int(123) +int(7) +bool(false) +int(1) +bool(true) +float(123.45) +string(3) "USD" +int(11) +bool(false) +NULL +int(1) +bool(true) From 4c71b4e58e37e8da533e5d86686e42fb0c70b019 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 18 Aug 2026 12:28:26 +0100 Subject: [PATCH 2/3] ext/standard: array_merge_recursive() fix leak. object to array conversion failure leaked the temporary zval. while at it, fix reverse expectation on a failed neighbour insertion. Close GH-23340 --- ext/standard/array.c | 3 +- .../array_merge_recursive_object_leak.phpt | 55 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 ext/standard/tests/array/array_merge_recursive_object_leak.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 85a017eff7f9..556f9a7ee666 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -4133,12 +4133,13 @@ PHPAPI int php_array_merge_recursive(HashTable *dest, HashTable *src) /* {{{ */ GC_TRY_UNPROTECT_RECURSION(thash); } if (!ret) { + zval_ptr_dtor(&tmp); return 0; } } else { Z_TRY_ADDREF_P(src_zval); zval *zv = zend_hash_next_index_insert(Z_ARRVAL_P(dest_zval), src_zval); - if (EXPECTED(!zv)) { + if (UNEXPECTED(!zv)) { Z_TRY_DELREF_P(src_zval); zend_cannot_add_element(); return 0; diff --git a/ext/standard/tests/array/array_merge_recursive_object_leak.phpt b/ext/standard/tests/array/array_merge_recursive_object_leak.phpt new file mode 100644 index 000000000000..f4313057cf81 --- /dev/null +++ b/ext/standard/tests/array/array_merge_recursive_object_leak.phpt @@ -0,0 +1,55 @@ +--TEST-- +array_merge_recursive() must not leak the array converted from an object when the merge below it fails +--FILE-- + (object) ['k' => 1]]); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +/* Control: same failing exit, array source, nothing to release. */ +$control = []; +$control['k'] = &$control; +try { + array_merge_recursive($control, ['k' => ['k' => 1]]); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +/* Several nested levels convert an object before the failure unwinds through them. */ +$ring = [[], [], []]; +for ($i = 0; $i < 3; $i++) { + $ring[$i]['k'] = &$ring[($i + 1) % 3]; +} +$src = (object) ['k' => 1]; +for ($i = 1; $i < 3; $i++) { + $src = (object) ['k' => $src]; +} +try { + array_merge_recursive($ring[0], ['k' => $src]); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +/* The successful path still releases it exactly once. */ +$ok = ['k' => ['a']]; +var_dump(array_merge_recursive($ok, ['k' => (object) ['b']])); + +?> +--EXPECT-- +Error: Recursion detected +Error: Recursion detected +Error: Recursion detected +array(1) { + ["k"]=> + array(2) { + [0]=> + string(1) "a" + [1]=> + string(1) "b" + } +} From 9256df25d52703219f431e929c6b6ac3f8bc9388 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 18 Aug 2026 12:29:34 +0100 Subject: [PATCH 3/3] [skip ci] Add NEWS entry --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 53bb8f7b1fb2..5ab1602dbd69 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,10 @@ PHP NEWS - Opcache: . Fixed opcache.protect_memory race under ZTS. (realFlowControl) +- Standard: + . Fixed a memory leak in array_merge_recursive() when the recursive merge of + an object converted to an array fails. (David Carlier) + - Zip: . Fixed ZipArchive::extractTo() and ZipArchive::getFrom*() reporting success on corrupted entries. (David Carlier)