From c4074d12cd5d1a14e99ab58c767f465913aed30d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 10 Aug 2026 12:07:40 +0100 Subject: [PATCH 1/3] ext/intl: IntlNumberRangeFormatter::format() crash when the formatting fails. When formatFormattableRange() failed, the error was set (which throws, as exceptions are force-enabled there) but execution fell through to intl_charFromString(), which returns NULL for the bogus result, and the NULL zend_string ended up in return_value as an IS_STRING zval. The engine then dereferenced it while discarding the return value. The conversion is now only attempted for a successful formatting and both failure paths return early. createFromSkeleton() had the same shape of defect without the crash: the skeleton failure path threw and then still built a LocalizedNumberRangeFormatter out of the failed skeleton, so it bails out early as well. --- .../rangeformatter/rangeformatter_class.cpp | 20 +++++-- .../rangeformatter_format_failure.phpt | 54 +++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 ext/intl/tests/rangeformatter/rangeformatter_format_failure.phpt diff --git a/ext/intl/rangeformatter/rangeformatter_class.cpp b/ext/intl/rangeformatter/rangeformatter_class.cpp index 95acfccd2452..5c0a82613b26 100644 --- a/ext/intl/rangeformatter/rangeformatter_class.cpp +++ b/ext/intl/rangeformatter/rangeformatter_class.cpp @@ -138,6 +138,8 @@ U_CFUNC PHP_METHOD(IntlNumberRangeFormatter, createFromSkeleton) INTL_G(use_exceptions) = old_use_exception; INTL_G(error_level) = old_error_level; + + RETURN_THROWS(); } LocalizedNumberRangeFormatter* nrf = new LocalizedNumberRangeFormatter( @@ -183,19 +185,29 @@ U_CFUNC PHP_METHOD(IntlNumberRangeFormatter, format) INTL_G(use_exceptions) = true; INTL_G(error_level) = 0; + zend_string *ret = NULL; + if (U_FAILURE(error)) { intl_errors_set(RANGEFORMATTER_ERROR_P(obj), error, "Failed to format number range"); - } + } else { + ret = intl_charFromString(result, &error); - zend_string *ret = intl_charFromString(result, &error); + if (UNEXPECTED(ret == NULL)) { + if (U_SUCCESS(error)) { + error = U_ILLEGAL_ARGUMENT_ERROR; + } - if (U_FAILURE(error)) { - intl_errors_set(RANGEFORMATTER_ERROR_P(obj), error, "Failed to convert result to UTF-8"); + intl_errors_set(RANGEFORMATTER_ERROR_P(obj), error, "Failed to convert result to UTF-8"); + } } INTL_G(use_exceptions) = old_use_exception; INTL_G(error_level) = old_error_level; + if (UNEXPECTED(ret == NULL)) { + RETURN_THROWS(); + } + RETVAL_NEW_STR(ret); } diff --git a/ext/intl/tests/rangeformatter/rangeformatter_format_failure.phpt b/ext/intl/tests/rangeformatter/rangeformatter_format_failure.phpt new file mode 100644 index 000000000000..854d0f74b995 --- /dev/null +++ b/ext/intl/tests/rangeformatter/rangeformatter_format_failure.phpt @@ -0,0 +1,54 @@ +--TEST-- +IntlNumberRangeFormatter::format() with a failing formatter +--EXTENSIONS-- +intl +--SKIPIF-- + +--FILE-- +format(1, 2); +} catch (IntlException $exception) { + echo $exception::class, ': ', $exception->getMessage(), PHP_EOL; +} + +var_dump($formatter->getErrorCode() !== 0); +var_dump(str_starts_with( + $formatter->getErrorMessage(), + 'IntlNumberRangeFormatter::format(): Failed to format number range: ' +)); + +var_dump(intl_get_error_code() !== 0); + +$formatter = IntlNumberRangeFormatter::createFromSkeleton( + '', + 'en_US', + IntlNumberRangeFormatter::COLLAPSE_AUTO, + IntlNumberRangeFormatter::IDENTITY_FALLBACK_RANGE +); + +var_dump($formatter->format(1, 2) !== ''); +var_dump($formatter->getErrorCode()); +var_dump($formatter->getErrorMessage()); + +?> +--EXPECT-- +IntlException: IntlNumberRangeFormatter::format(): Failed to format number range +bool(true) +bool(true) +bool(true) +bool(true) +int(0) +string(12) "U_ZERO_ERROR" From 9aa98ed75e3c4e51dd50bb0995f69002c3c0e6e4 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 10 Aug 2026 11:58:17 +0100 Subject: [PATCH 2/3] IntlNumberRangeFormatter error state reset cleanup. format() reset the global error slot and the object one separately, and both methods reset before parsing their parameters, so a TypeError also cleared the state. Use intl_errors_reset(), which covers both slots, and reset once the parameters are known to be good. --- .../rangeformatter/rangeformatter_class.cpp | 11 ++-- .../rangeformatter_error_reset_scope.phpt | 64 +++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt diff --git a/ext/intl/rangeformatter/rangeformatter_class.cpp b/ext/intl/rangeformatter/rangeformatter_class.cpp index 5c0a82613b26..37b49e4f1310 100644 --- a/ext/intl/rangeformatter/rangeformatter_class.cpp +++ b/ext/intl/rangeformatter/rangeformatter_class.cpp @@ -88,8 +88,6 @@ U_CFUNC PHP_METHOD(IntlNumberRangeFormatter, createFromSkeleton) zend_long collapse; zend_long identityFallback; - intl_error_reset(NULL); - ZEND_PARSE_PARAMETERS_START(4,4) Z_PARAM_STRING(skeleton, skeleton_len) Z_PARAM_STRING(locale, locale_len) @@ -97,6 +95,8 @@ U_CFUNC PHP_METHOD(IntlNumberRangeFormatter, createFromSkeleton) Z_PARAM_LONG(identityFallback) ZEND_PARSE_PARAMETERS_END(); + intl_error_reset(NULL); + if (locale_len == 0) { locale = (char *)intl_locale_get_default(); } @@ -162,16 +162,17 @@ U_CFUNC PHP_METHOD(IntlNumberRangeFormatter, format) zval *start; zval *end; - intl_error_reset(NULL); - IntlNumberRangeFormatter_object* obj = Z_INTL_RANGEFORMATTER_P(ZEND_THIS); - intl_error_reset(RANGEFORMATTER_ERROR_P(obj)); ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_NUMBER(start) Z_PARAM_NUMBER(end) ZEND_PARSE_PARAMETERS_END(); + intl_errors_reset(RANGEFORMATTER_ERROR_P(obj)); + + ZEND_ASSERT(RANGEFORMATTER_OBJECT(obj) != NULL); + UErrorCode error = U_ZERO_ERROR; icu::Formattable start_formattable = rangeformatter_create_formattable(start); diff --git a/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt b/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt new file mode 100644 index 000000000000..89631a75f97e --- /dev/null +++ b/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt @@ -0,0 +1,64 @@ +--TEST-- +IntlNumberRangeFormatter keeps the intl error state on a parameter error +--EXTENSIONS-- +intl +--SKIPIF-- + +--FILE-- +format([], 2); +} catch (TypeError $error) { + echo $error->getMessage(), PHP_EOL; +} + +var_dump(intl_get_error_code() !== 0); + +try { + IntlNumberRangeFormatter::createFromSkeleton( + [], + 'en_US', + IntlNumberRangeFormatter::COLLAPSE_AUTO, + IntlNumberRangeFormatter::IDENTITY_FALLBACK_RANGE + ); +} catch (TypeError $error) { + echo $error->getMessage(), PHP_EOL; +} + +var_dump(intl_get_error_code() !== 0); + +$formatter->format(1, 2); + +var_dump(intl_get_error_code()); +var_dump($formatter->getErrorCode()); + +?> +--EXPECT-- +IntlNumberRangeFormatter::format(): Argument #1 ($start) must be of type int|float, array given +bool(true) +IntlNumberRangeFormatter::createFromSkeleton(): Argument #1 ($skeleton) must be of type string, array given +bool(true) +int(0) +int(0) From 5ec694a69cd2f16a5a5a6e8425814a55017b5502 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 10 Aug 2026 14:47:13 +0100 Subject: [PATCH 3/3] feedback --- .../rangeformatter/rangeformatter_error_reset_scope.phpt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt b/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt index 89631a75f97e..7fcd37a00c69 100644 --- a/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt +++ b/ext/intl/tests/rangeformatter/rangeformatter_error_reset_scope.phpt @@ -31,7 +31,7 @@ try { try { $formatter->format([], 2); } catch (TypeError $error) { - echo $error->getMessage(), PHP_EOL; + echo $error::class, ': ', $error->getMessage(), PHP_EOL; } var_dump(intl_get_error_code() !== 0); @@ -44,7 +44,7 @@ try { IntlNumberRangeFormatter::IDENTITY_FALLBACK_RANGE ); } catch (TypeError $error) { - echo $error->getMessage(), PHP_EOL; + echo $error::class, ': ', $error->getMessage(), PHP_EOL; } var_dump(intl_get_error_code() !== 0); @@ -56,9 +56,9 @@ var_dump($formatter->getErrorCode()); ?> --EXPECT-- -IntlNumberRangeFormatter::format(): Argument #1 ($start) must be of type int|float, array given +TypeError: IntlNumberRangeFormatter::format(): Argument #1 ($start) must be of type int|float, array given bool(true) -IntlNumberRangeFormatter::createFromSkeleton(): Argument #1 ($skeleton) must be of type string, array given +TypeError: IntlNumberRangeFormatter::createFromSkeleton(): Argument #1 ($skeleton) must be of type string, array given bool(true) int(0) int(0)