[php-src] PHP-8.4: Keep compiled RuleBasedBreakIterator rules alive for the iterator

Ilia Alshanetsky <[email protected]>
Newsgroups gmane.comp.php.cvs.general
Message-ID <[email protected]>
Author: Ilia Alshanetsky (iliaal)
Date: 2026-08-28T09:11:50-04:00

Commit: https://github.com/php/php-src/commit/35b09eaead26bbe865bc26b0d1639ef238228863
Raw diff: https://github.com/php/php-src/commit/35b09eaead26bbe865bc26b0d1639ef238228863.diff

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.

Closes GH-23322

Changed paths:
  A  ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt
  M  NEWS
  M  ext/intl/breakiterator/breakiterator_class.cpp
  M  ext/intl/breakiterator/breakiterator_class.h
  M  ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp


Diff:

diff --git a/NEWS b/NEWS
index cf28751877fa..519b0ccaf053 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,8 @@ PHP                                                                        NEWS
   . Fixed grapheme_str_split() treating UBRK_DONE as a byte index. (iliaal)
   . Fixed a leak in Locale::getKeywords() when a keyword value cannot be
     read. (iliaal)
+  . 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<uint8_t *>(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..d0eb5fc2e755
--- /dev/null
+++ b/ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt
@@ -0,0 +1,77 @@
+--TEST--
+IntlRuleBasedBreakIterator compiled rules outlive the source string
+--EXTENSIONS--
+intl
+--SKIPIF--
+<?php if (version_compare(INTL_ICU_VERSION, '68.1') < 0) die('skip for ICU >= 68.1'); ?>
+--FILE--
+<?php
+
+$rules = <<<RULES
+\$LN = [[:letter:] [:number:]];
+\$S = [.;,:];
+
+!!forward;
+\$LN+ {1};
+\$S+ {42};
+!!reverse;
+\$LN+ {1};
+\$S+ {42};
+!!safe_forward;
+!!safe_reverse;
+RULES;
+
+$src = new IntlRuleBasedBreakIterator($rules);
+$len = strlen($src->getBinaryRules());
+
+$it = new IntlRuleBasedBreakIterator($src->getBinaryRules(), true);
+unset($src);
+
+/* ICU aliases the buffer it was built from, so the freed rules have to be
+   reclaimed and overwritten for the iterator below to read stale bytes. */
+$ballast = [];
+for ($i = 0; $i < 16; $i++) {
+    $ballast[] = str_repeat("\xCC", $len);
+}
+
+$it->setText('ab,cd');
+echo $it->first(), "\n";
+while (true) {
+    $n = $it->next();
+    if ($n === IntlBreakIterator::DONE) {
+        break;
+    }
+    echo $n, "\n";
+}
+
+$clone = clone $it;
+unset($it);
+$ballast[] = str_repeat("\xDD", $len);
+$clone->setText('xy');
+echo $clone->first(), "\n";
+echo $clone->next(), "\n";
+
+$src = new IntlRuleBasedBreakIterator($rules);
+$it = new IntlRuleBasedBreakIterator($src->getBinaryRules(), true);
+unset($src);
+for ($i = 0; $i < 16; $i++) {
+    $ballast[] = str_repeat("\xEE", $len);
+}
+$it->setText('ab,cd');
+$parts = $it->getPartsIterator();
+unset($it);
+foreach ($parts as $p) {
+    echo $p, "\n";
+}
+
+?>
+--EXPECT--
+0
+2
+3
+5
+0
+2
+ab
+,
+cd
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.