diff --git a/NEWS b/NEWS index 338409dd70c5..d18e3a421b76 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,10 @@ PHP NEWS . Fixed a use-after-free when cloning a DOMNameSpaceNode after DOMDocument::xinclude(). (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..42924ac97654 --- /dev/null +++ b/ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt @@ -0,0 +1,61 @@ +--TEST-- +IntlRuleBasedBreakIterator compiled rules outlive the source string +--EXTENSIONS-- +intl +--SKIPIF-- += 68.1'); ?> +--FILE-- +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"; + +?> +--EXPECT-- +0 +2 +3 +5 +0 +2