Skip to content

Commit 6e871a7

Browse files
committed
Keep compiled RuleBasedBreakIterator rules alive for the iterator
The ICU compiled-rules constructor aliases the caller's buffer (kDontAdopt). PHP passed ZSTR_VAL of a temporary 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.
1 parent e5623ea commit 6e871a7

5 files changed

Lines changed: 67 additions & 0 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ PHP NEWS
2020
left busy for the next fetch, and rows delivered from a result another
2121
statement took over. (KentarouTakeda)
2222

23+
- Intl:
24+
. Fixed a use-after-free when IntlRuleBasedBreakIterator is constructed
25+
from compiled rules and the source string is released. (iliaal)
26+
2327
- Phar:
2428
. Fixed Phar archives being automatically detected when ".phar" only occurs
2529
in a directory name or is not a filename extension in an included file's

ext/intl/breakiterator/breakiterator_class.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ static zend_object *BreakIterator_clone_obj(zend_object *object)
109109
} else {
110110
bio_new->biter = new_biter;
111111
ZVAL_COPY(&bio_new->text, &bio_orig->text);
112+
if (bio_orig->compiled_rules) {
113+
bio_new->compiled_rules = zend_string_copy(bio_orig->compiled_rules);
114+
}
112115
}
113116
} else {
114117
zend_throw_error(NULL, "Cannot clone uninitialized BreakIterator");
@@ -163,6 +166,7 @@ static void breakiterator_object_init(BreakIterator_object *bio)
163166
{
164167
intl_error_init(BREAKITER_ERROR_P(bio));
165168
bio->biter = NULL;
169+
bio->compiled_rules = NULL;
166170
ZVAL_UNDEF(&bio->text);
167171
}
168172
/* }}} */
@@ -177,6 +181,10 @@ static void BreakIterator_objects_free(zend_object *object)
177181
delete bio->biter;
178182
bio->biter = NULL;
179183
}
184+
if (bio->compiled_rules) {
185+
zend_string_release(bio->compiled_rules);
186+
bio->compiled_rules = NULL;
187+
}
180188
intl_error_reset(BREAKITER_ERROR_P(bio));
181189

182190
zend_object_std_dtor(&bio->zo);

ext/intl/breakiterator/breakiterator_class.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ typedef struct {
3838
// current text
3939
zval text;
4040

41+
zend_string *compiled_rules;
42+
4143
zend_object zo;
4244
} BreakIterator_object;
4345

ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, __construct)
8787
}
8888

8989
breakiterator_object_create(object, rbbi, false);
90+
if (compiled) {
91+
Z_INTL_BREAKITERATOR_P(object)->compiled_rules = zend_string_copy(rules);
92+
}
9093
}
9194

9295
U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, getRules)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
IntlRuleBasedBreakIterator compiled rules outlive the source string
3+
--EXTENSIONS--
4+
intl
5+
--SKIPIF--
6+
<?php if (version_compare(INTL_ICU_VERSION, '68.1') < 0) die('skip for ICU >= 68.1'); ?>
7+
--FILE--
8+
<?php
9+
10+
$rules = <<<RULES
11+
\$LN = [[:letter:] [:number:]];
12+
\$S = [.;,:];
13+
14+
!!forward;
15+
\$LN+ {1};
16+
\$S+ {42};
17+
!!reverse;
18+
\$LN+ {1};
19+
\$S+ {42};
20+
!!safe_forward;
21+
!!safe_reverse;
22+
RULES;
23+
24+
$src = new IntlRuleBasedBreakIterator($rules);
25+
$it = new IntlRuleBasedBreakIterator($src->getBinaryRules(), true);
26+
unset($src);
27+
28+
$it->setText('ab,cd');
29+
echo $it->first(), "\n";
30+
while (true) {
31+
$n = $it->next();
32+
if ($n === IntlBreakIterator::DONE) {
33+
break;
34+
}
35+
echo $n, "\n";
36+
}
37+
38+
$clone = clone $it;
39+
$clone->setText('xy');
40+
echo $clone->first(), "\n";
41+
echo $clone->next(), "\n";
42+
43+
?>
44+
--EXPECT--
45+
0
46+
2
47+
3
48+
5
49+
0
50+
2

0 commit comments

Comments
 (0)