From 7678a8704bb28169af8b6e70ec8cc2a668b3cfce Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 16 Aug 2026 12:48:37 -0400 Subject: [PATCH] Keep compiled RuleBasedBreakIterator rules alive for the iterator The ICU compiled-rules constructor aliases the caller's buffer. PHP passed the argument string and did not retain it, so a later setText/next can use freed memory. Hold a zend_string copy on the object and release it in free_obj; clone addrefs it. --- NEWS | 4 ++ .../breakiterator/breakiterator_class.cpp | 8 +++ ext/intl/breakiterator/breakiterator_class.h | 2 + .../rulebasedbreakiterator_methods.cpp | 12 +++-- .../rbbiter_compiled_rules_lifetime.phpt | 50 +++++++++++++++++++ 5 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt diff --git a/NEWS b/NEWS index ecb4d105e917..fc0469bf7aa9 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ PHP NEWS . Fixed bug GH-15375 (Nested "yield from" skips items after a valid() or next() call on the inner generator). (iliaal) +- Intl: + . Fixed a use-after-free when IntlRuleBasedBreakIterator is constructed + from compiled rules. (iliaal) + - Opcache: . Fixed opcache.protect_memory race under ZTS. (realFlowControl) diff --git a/ext/intl/breakiterator/breakiterator_class.cpp b/ext/intl/breakiterator/breakiterator_class.cpp index 4976d4ff675b..e078ff691274 100644 --- a/ext/intl/breakiterator/breakiterator_class.cpp +++ b/ext/intl/breakiterator/breakiterator_class.cpp @@ -109,6 +109,9 @@ static zend_object *BreakIterator_clone_obj(zend_object *object) } else { bio_new->biter = new_biter; ZVAL_COPY(&bio_new->text, &bio_orig->text); + if (bio_orig->compiled_rules) { + bio_new->compiled_rules = zend_string_copy(bio_orig->compiled_rules); + } } } else { zend_throw_error(NULL, "Cannot clone uninitialized BreakIterator"); @@ -163,6 +166,7 @@ static void breakiterator_object_init(BreakIterator_object *bio) { intl_error_init(BREAKITER_ERROR_P(bio)); bio->biter = NULL; + bio->compiled_rules = NULL; ZVAL_UNDEF(&bio->text); } /* }}} */ @@ -177,6 +181,10 @@ static void BreakIterator_objects_free(zend_object *object) delete bio->biter; bio->biter = NULL; } + if (bio->compiled_rules) { + zend_string_release(bio->compiled_rules); + bio->compiled_rules = NULL; + } intl_error_reset(BREAKITER_ERROR_P(bio)); zend_object_std_dtor(&bio->zo); diff --git a/ext/intl/breakiterator/breakiterator_class.h b/ext/intl/breakiterator/breakiterator_class.h index 0852d86e2a82..8061acc0ddac 100644 --- a/ext/intl/breakiterator/breakiterator_class.h +++ b/ext/intl/breakiterator/breakiterator_class.h @@ -38,6 +38,8 @@ typedef struct { // current text zval text; + zend_string *compiled_rules; + zend_object zo; } BreakIterator_object; diff --git a/ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp b/ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp index c84972fe5b98..75a52b6047d8 100644 --- a/ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp +++ b/ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp @@ -34,15 +34,14 @@ static inline RuleBasedBreakIterator *fetch_rbbi(BreakIterator_object *bio) { static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_error_handling *error_handling, bool *error_handling_replaced) { - char *rules; - size_t rules_len; + zend_string *rules; bool compiled = false; UErrorCode status = U_ZERO_ERROR; BREAKITER_METHOD_INIT_VARS; object = ZEND_THIS; ZEND_PARSE_PARAMETERS_START(1, 2) - Z_PARAM_STRING(rules, rules_len) + Z_PARAM_STR(rules) Z_PARAM_OPTIONAL Z_PARAM_BOOL(compiled) ZEND_PARSE_PARAMETERS_END(); @@ -62,7 +61,7 @@ static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_er if (!compiled) { UnicodeString rulesStr; UParseError parseError = UParseError(); - if (intl_stringFromChar(rulesStr, rules, rules_len, &status) + if (intl_stringFromChar(rulesStr, ZSTR_VAL(rules), ZSTR_LEN(rules), &status) == FAILURE) { zend_throw_exception(IntlException_ce_ptr, "IntlRuleBasedBreakIterator::__construct(): " @@ -84,7 +83,7 @@ static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_er RETURN_THROWS(); } } else { // compiled - rbbi = new RuleBasedBreakIterator((uint8_t*)rules, rules_len, status); + rbbi = new RuleBasedBreakIterator(reinterpret_cast(ZSTR_VAL(rules)), ZSTR_LEN(rules), status); if (U_FAILURE(status)) { zend_throw_exception(IntlException_ce_ptr, "IntlRuleBasedBreakIterator::__construct(): " @@ -95,6 +94,9 @@ static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_er } breakiterator_object_create(return_value, rbbi, 0); + if (compiled) { + Z_INTL_BREAKITERATOR_P(return_value)->compiled_rules = zend_string_copy(rules); + } } U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, __construct) diff --git a/ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt b/ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt new file mode 100644 index 000000000000..c2d9ba9c51a7 --- /dev/null +++ b/ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt @@ -0,0 +1,50 @@ +--TEST-- +IntlRuleBasedBreakIterator compiled rules outlive the source string +--EXTENSIONS-- +intl +--SKIPIF-- += 68.1'); ?> +--FILE-- +getBinaryRules(), true); +unset($src); + +$it->setText('ab,cd'); +echo $it->first(), "\n"; +while (true) { + $n = $it->next(); + if ($n === IntlBreakIterator::DONE) { + break; + } + echo $n, "\n"; +} + +$clone = clone $it; +$clone->setText('xy'); +echo $clone->first(), "\n"; +echo $clone->next(), "\n"; + +?> +--EXPECT-- +0 +2 +3 +5 +0 +2