From afa671366fc24844a4badc5944b46e9b51f53fed Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 10:51:55 -0400 Subject: [PATCH] ext/intl: Reject unconstructed Collator in attribute and strength methods Collator::getAttribute(), setAttribute(), getStrength() and setStrength() dereferenced a NULL ICU collator when called on an object whose constructor skipped parent::__construct(), returning bogus values instead of failing, while compare(), getLocale(), sort() and getSortKey() already throw "Object not initialized". Apply the same guard to the four remaining methods through a collator_check_initialized() helper, which also replaces four existing copies. Closes GH-23797 --- NEWS | 2 + ext/intl/collator/collator_attr.c | 16 ++++++ ext/intl/collator/collator_class.h | 16 ++++++ ext/intl/collator/collator_compare.c | 7 +-- ext/intl/collator/collator_locale.c | 7 +-- ext/intl/collator/collator_sort.c | 14 +---- .../collator_attribute_unconstructed.phpt | 55 +++++++++++++++++++ 7 files changed, 93 insertions(+), 24 deletions(-) create mode 100644 ext/intl/tests/collator_attribute_unconstructed.phpt diff --git a/NEWS b/NEWS index 982945e5ebfc..826b6e86531e 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,8 @@ PHP NEWS . Fixed cloning IntlDateFormatter and MessageFormatter losing PHP-side state such as dateType, timeType, calendar and the message pattern. (Ilia Alshanetsky) + . Fixed Collator attribute and strength methods not rejecting an + unconstructed Collator. (Ilia Alshanetsky) - Lexbor: . Merge patches lexbor/lexbor@8a14bc0 and lexbor/lexbor@f67ce4b, fixing a diff --git a/ext/intl/collator/collator_attr.c b/ext/intl/collator/collator_attr.c index f16ae0cc5285..0bc680260928 100644 --- a/ext/intl/collator/collator_attr.c +++ b/ext/intl/collator/collator_attr.c @@ -40,6 +40,10 @@ PHP_FUNCTION( collator_get_attribute ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; + if (collator_check_initialized(co) == FAILURE) { + RETURN_THROWS(); + } + value = ucol_getAttribute( co->ucoll, attribute, COLLATOR_ERROR_CODE_P( co ) ); COLLATOR_CHECK_STATUS( co, "Error getting attribute value" ); @@ -64,6 +68,10 @@ PHP_FUNCTION( collator_set_attribute ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; + if (collator_check_initialized(co) == FAILURE) { + RETURN_THROWS(); + } + /* Set new value for the given attribute. */ ucol_setAttribute( co->ucoll, attribute, value, COLLATOR_ERROR_CODE_P( co ) ); COLLATOR_CHECK_STATUS( co, "Error setting attribute value" ); @@ -87,6 +95,10 @@ PHP_FUNCTION( collator_get_strength ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; + if (collator_check_initialized(co) == FAILURE) { + RETURN_THROWS(); + } + /* Get current strength and return it. */ RETURN_LONG( ucol_getStrength( co->ucoll ) ); } @@ -109,6 +121,10 @@ PHP_FUNCTION( collator_set_strength ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; + if (collator_check_initialized(co) == FAILURE) { + RETURN_THROWS(); + } + /* Set given strength. */ ucol_setStrength( co->ucoll, strength ); diff --git a/ext/intl/collator/collator_class.h b/ext/intl/collator/collator_class.h index 5c69c2e5affb..9fdfbb0d01bd 100644 --- a/ext/intl/collator/collator_class.h +++ b/ext/intl/collator/collator_class.h @@ -46,6 +46,22 @@ static inline Collator_object *php_intl_collator_fetch_object(zend_object *obj) } #define Z_INTL_COLLATOR_P(zv) php_intl_collator_fetch_object(Z_OBJ_P(zv)) +static zend_always_inline zend_result collator_check_initialized(Collator_object *co) +{ + ZEND_ASSERT(co != NULL); + + if (UNEXPECTED(co->ucoll == NULL)) { + intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) ); + intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ), + "Object not initialized", 0 ); + zend_throw_error(NULL, "Object not initialized"); + + return FAILURE; + } + + return SUCCESS; +} + void collator_register_Collator_symbols(int module_number); void collator_object_init( Collator_object* co ); void collator_object_destroy( Collator_object* co ); diff --git a/ext/intl/collator/collator_compare.c b/ext/intl/collator/collator_compare.c index f71d57f74f86..26d05601f94d 100644 --- a/ext/intl/collator/collator_compare.c +++ b/ext/intl/collator/collator_compare.c @@ -48,12 +48,7 @@ PHP_FUNCTION( collator_compare ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; - if (!co || !co->ucoll) { - intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) ); - intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ), - "Object not initialized", 0 ); - zend_throw_error(NULL, "Object not initialized"); - + if (collator_check_initialized(co) == FAILURE) { RETURN_THROWS(); } diff --git a/ext/intl/collator/collator_locale.c b/ext/intl/collator/collator_locale.c index e1cdcdf2a609..22c6e672a5ac 100644 --- a/ext/intl/collator/collator_locale.c +++ b/ext/intl/collator/collator_locale.c @@ -41,12 +41,7 @@ PHP_FUNCTION( collator_get_locale ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; - if (!co || !co->ucoll) { - intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) ); - intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ), - "Object not initialized", 0 ); - zend_throw_error(NULL, "Object not initialized"); - + if (collator_check_initialized(co) == FAILURE) { RETURN_THROWS(); } diff --git a/ext/intl/collator/collator_sort.c b/ext/intl/collator/collator_sort.c index 9d4cb4220203..6b8feaaa03f5 100644 --- a/ext/intl/collator/collator_sort.c +++ b/ext/intl/collator/collator_sort.c @@ -376,12 +376,7 @@ PHP_FUNCTION( collator_sort_with_sort_keys ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; - if (!co || !co->ucoll) { - intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) ); - intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ), - "Object not initialized", 0 ); - zend_throw_error(NULL, "Object not initialized"); - + if (collator_check_initialized(co) == FAILURE) { RETURN_THROWS(); } @@ -532,12 +527,7 @@ PHP_FUNCTION( collator_get_sort_key ) /* Fetch the object. */ COLLATOR_METHOD_FETCH_OBJECT; - if (!co || !co->ucoll) { - intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) ); - intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ), - "Object not initialized", 0 ); - zend_throw_error(NULL, "Object not initialized"); - + if (collator_check_initialized(co) == FAILURE) { RETURN_THROWS(); } diff --git a/ext/intl/tests/collator_attribute_unconstructed.phpt b/ext/intl/tests/collator_attribute_unconstructed.phpt new file mode 100644 index 000000000000..f8eb5afa01d5 --- /dev/null +++ b/ext/intl/tests/collator_attribute_unconstructed.phpt @@ -0,0 +1,55 @@ +--TEST-- +Collator attribute and strength methods on unconstructed object +--EXTENSIONS-- +intl +--FILE-- + fn() => $c->getAttribute(Collator::NUMERIC_COLLATION), + 'setAttribute' => fn() => $c->setAttribute(Collator::NUMERIC_COLLATION, Collator::ON), + 'getStrength' => fn() => $c->getStrength(), + 'setStrength' => fn() => $c->setStrength(Collator::SECONDARY), +]; + +foreach ($methods as $method => $call) { + try { + $call(); + } catch (Error $e) { + echo $method, ': ', $e::class, ': ', $e->getMessage(), PHP_EOL; + } +} + +$functions = [ + 'collator_get_attribute' => fn() => collator_get_attribute($c, Collator::NUMERIC_COLLATION), + 'collator_set_attribute' => fn() => collator_set_attribute($c, Collator::NUMERIC_COLLATION, Collator::ON), + 'collator_get_strength' => fn() => collator_get_strength($c), + 'collator_set_strength' => fn() => collator_set_strength($c, Collator::SECONDARY), +]; + +foreach ($functions as $function => $call) { + try { + $call(); + } catch (Error $e) { + echo $function, ': ', $e::class, ': ', $e->getMessage(), PHP_EOL; + } +} + +?> +--EXPECT-- +getAttribute: Error: Object not initialized +setAttribute: Error: Object not initialized +getStrength: Error: Object not initialized +setStrength: Error: Object not initialized +collator_get_attribute: Error: Object not initialized +collator_set_attribute: Error: Object not initialized +collator_get_strength: Error: Object not initialized +collator_set_strength: Error: Object not initialized