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.

Author: iliaal

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)
 

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);

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;
 

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)

ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt

@@ -0,0 +1,50 @@
+--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);
+$it = new IntlRuleBasedBreakIterator($src->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