Skip to content

Commit 92e2fd6

Browse files
iliaaliliaal
committed
Merge branch 'PHP-8.4' into PHP-8.5
* PHP-8.4: ext/tidy: Reject tidyNode use after the document is reparsed
2 parents 6030e56 + 30ba021 commit 92e2fd6

3 files changed

Lines changed: 113 additions & 11 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ PHP NEWS
163163
lookup, and an unchecked CreateFileA() failure. (Ilia Alshanetsky)
164164
. Fix persistent stream context lifetime during shutdown (Levi Morrison)
165165

166+
- Tidy:
167+
. Fixed a use-after-free when a tidyNode is used after its document is
168+
reparsed. (Ilia Alshanetsky)
169+
166170
- Zlib:
167171
. Fixed inflate_init() dropping the preset dictionary for raw streams with
168172
a non-default window. (Ilia Alshanetsky)

‎ext/tidy/tests/reparse_node.phpt‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--TEST--
2+
tidyNode objects are invalid after reparsing their document
3+
--EXTENSIONS--
4+
tidy
5+
--FILE--
6+
<?php
7+
8+
$node = unserialize('O:8:"tidyNode":0:{}');
9+
try {
10+
$node->hasChildren();
11+
echo "unowned node: no error\n";
12+
} catch (Error $e) {
13+
echo 'unowned node: ', $e::class, ': ', $e->getMessage(), "\n";
14+
}
15+
16+
$tidy = tidy_parse_string('<html><body><p>one</p><p>two</p></body></html>');
17+
$node = $tidy->body()->child[0];
18+
var_dump($node->isHtml());
19+
var_dump($node->hasSiblings());
20+
21+
$tidy->parseString('<html><body><p>three</p></body></html>');
22+
23+
$operations = [
24+
'string cast' => static fn() => (string) $node,
25+
'hasChildren' => static fn() => $node->hasChildren(),
26+
'hasSiblings' => static fn() => $node->hasSiblings(),
27+
'isComment' => static fn() => $node->isComment(),
28+
'isHtml' => static fn() => $node->isHtml(),
29+
'isText' => static fn() => $node->isText(),
30+
'isJste' => static fn() => $node->isJste(),
31+
'isAsp' => static fn() => $node->isAsp(),
32+
'isPhp' => static fn() => $node->isPhp(),
33+
'getParent' => static fn() => $node->getParent(),
34+
'getPreviousSibling' => static fn() => $node->getPreviousSibling(),
35+
'getNextSibling' => static fn() => $node->getNextSibling(),
36+
];
37+
38+
foreach ($operations as $operation => $callback) {
39+
try {
40+
$callback();
41+
echo $operation, ": no error\n";
42+
} catch (Error $e) {
43+
echo $operation, ': ', $e::class, ': ', $e->getMessage(), "\n";
44+
}
45+
}
46+
47+
var_dump($tidy->body()->child[0]->isHtml());
48+
49+
?>
50+
--EXPECT--
51+
unowned node: Error: tidyNode object is not initialized
52+
bool(true)
53+
bool(true)
54+
string cast: Error: tidyNode object is no longer valid after its document was reparsed
55+
hasChildren: Error: tidyNode object is no longer valid after its document was reparsed
56+
hasSiblings: Error: tidyNode object is no longer valid after its document was reparsed
57+
isComment: Error: tidyNode object is no longer valid after its document was reparsed
58+
isHtml: Error: tidyNode object is no longer valid after its document was reparsed
59+
isText: Error: tidyNode object is no longer valid after its document was reparsed
60+
isJste: Error: tidyNode object is no longer valid after its document was reparsed
61+
isAsp: Error: tidyNode object is no longer valid after its document was reparsed
62+
isPhp: Error: tidyNode object is no longer valid after its document was reparsed
63+
getParent: Error: tidyNode object is no longer valid after its document was reparsed
64+
getPreviousSibling: Error: tidyNode object is no longer valid after its document was reparsed
65+
getNextSibling: Error: tidyNode object is no longer valid after its document was reparsed
66+
bool(true)

‎ext/tidy/tidy.c‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@
7070
ZEND_PARSE_PARAMETERS_NONE(); \
7171
obj = Z_TIDY_P(ZEND_THIS); \
7272

73+
#define TIDY_FETCH_VALID_NODE \
74+
TIDY_FETCH_ONLY_OBJECT; \
75+
if (tidy_node_validate(obj) != SUCCESS) { \
76+
RETURN_THROWS(); \
77+
}
7378
#define TIDY_SET_DEFAULT_CONFIG(_doc) \
7479
if (TG(default_config) && TG(default_config)[0]) { \
7580
php_tidy_load_config(_doc, TG(default_config)); \
@@ -96,12 +101,14 @@ struct _PHPTidyDoc {
96101
TidyDoc doc;
97102
TidyBuffer *errbuf;
98103
uint32_t ref_count;
104+
size_t parse_generation;
99105
bool initialized;
100106
};
101107

102108
struct _PHPTidyObj {
103109
TidyNode node;
104110
tidy_obj_type type;
111+
size_t node_generation;
105112
PHPTidyDoc *ptdoc;
106113
zend_object std;
107114
};
@@ -215,12 +222,28 @@ static zend_result php_tidy_apply_config(TidyDoc doc, const zend_string *str_str
215222
return SUCCESS;
216223
}
217224

225+
static zend_result tidy_node_validate(const PHPTidyObj *obj)
226+
{
227+
if (!obj->ptdoc) {
228+
zend_throw_error(NULL, "tidyNode object is not initialized");
229+
return FAILURE;
230+
}
231+
232+
if (obj->node_generation != obj->ptdoc->parse_generation) {
233+
zend_throw_error(NULL, "tidyNode object is no longer valid after its document was reparsed");
234+
return FAILURE;
235+
}
236+
237+
return SUCCESS;
238+
}
239+
218240
static void tidy_create_node_object(zval *zv, PHPTidyDoc *ptdoc, TidyNode node)
219241
{
220242
object_init_ex(zv, tidy_ce_node);
221243
PHPTidyObj *newobj = Z_TIDY_P(zv);
222244
newobj->node = node;
223245
newobj->type = is_node;
246+
newobj->node_generation = ptdoc->parse_generation;
224247
newobj->ptdoc = ptdoc;
225248
newobj->ptdoc->ref_count++;
226249
tidy_add_node_default_properties(newobj);
@@ -378,6 +401,7 @@ static zend_object *tidy_object_new(zend_class_entry *class_type, const zend_obj
378401
intern->ptdoc = emalloc(sizeof(PHPTidyDoc));
379402
intern->ptdoc->doc = tidyCreate();
380403
intern->ptdoc->ref_count = 1;
404+
intern->ptdoc->parse_generation = 0;
381405
intern->ptdoc->initialized = false;
382406
intern->ptdoc->errbuf = emalloc(sizeof(TidyBuffer));
383407
tidyBufInit(intern->ptdoc->errbuf);
@@ -472,6 +496,9 @@ static zend_result tidy_node_cast_handler(zend_object *in, zval *out, int type)
472496

473497
case IS_STRING:
474498
obj = php_tidy_fetch_object(in);
499+
if (tidy_node_validate(obj) != SUCCESS) {
500+
return FAILURE;
501+
}
475502
tidyBufInit(&buf);
476503
if (obj->ptdoc && tidyNodeGetText(obj->ptdoc->doc, obj->node, &buf)) {
477504
ZVAL_STRINGL(out, (const char *) buf.bp, buf.size-1);
@@ -528,6 +555,10 @@ static void tidy_add_node_default_properties(PHPTidyObj *obj)
528555
zval attribute, children, temp;
529556
const char *name;
530557

558+
if (tidy_node_validate(obj) != SUCCESS) {
559+
return;
560+
}
561+
531562
tidyBufInit(&buf);
532563
(void) tidyNodeGetText(obj->ptdoc->doc, obj->node, &buf);
533564

@@ -816,6 +847,7 @@ static zend_result php_tidy_parse_string(PHPTidyObj *obj, const zend_string *str
816847
obj->ptdoc->initialized = true;
817848

818849
tidyBufInit(&buf);
850+
obj->ptdoc->parse_generation++;
819851
tidyBufAttach(&buf, (byte *) ZSTR_VAL(string), (unsigned int) ZSTR_LEN(string));
820852
if (tidyParseBuffer(obj->ptdoc->doc, &buf) < 0) {
821853
php_error_docref(NULL, E_WARNING, "%s", (const char*) obj->ptdoc->errbuf->bp);
@@ -1475,7 +1507,7 @@ PHP_FUNCTION(tidy_get_body)
14751507
/* {{{ Returns true if this node has children */
14761508
PHP_METHOD(tidyNode, hasChildren)
14771509
{
1478-
TIDY_FETCH_ONLY_OBJECT;
1510+
TIDY_FETCH_VALID_NODE;
14791511

14801512
RETURN_BOOL(tidyGetChild(obj->node));
14811513
}
@@ -1484,7 +1516,7 @@ PHP_METHOD(tidyNode, hasChildren)
14841516
/* {{{ Returns true if this node has siblings */
14851517
PHP_METHOD(tidyNode, hasSiblings)
14861518
{
1487-
TIDY_FETCH_ONLY_OBJECT;
1519+
TIDY_FETCH_VALID_NODE;
14881520

14891521
RETURN_BOOL(obj->node && tidyGetNext(obj->node));
14901522
}
@@ -1493,7 +1525,7 @@ PHP_METHOD(tidyNode, hasSiblings)
14931525
/* {{{ Returns true if this node represents a comment */
14941526
PHP_METHOD(tidyNode, isComment)
14951527
{
1496-
TIDY_FETCH_ONLY_OBJECT;
1528+
TIDY_FETCH_VALID_NODE;
14971529

14981530
RETURN_BOOL(tidyNodeGetType(obj->node) == TidyNode_Comment);
14991531
}
@@ -1502,7 +1534,7 @@ PHP_METHOD(tidyNode, isComment)
15021534
/* {{{ Returns true if this node is part of a HTML document */
15031535
PHP_METHOD(tidyNode, isHtml)
15041536
{
1505-
TIDY_FETCH_ONLY_OBJECT;
1537+
TIDY_FETCH_VALID_NODE;
15061538

15071539
switch (tidyNodeGetType(obj->node)) {
15081540
case TidyNode_Start:
@@ -1518,7 +1550,7 @@ PHP_METHOD(tidyNode, isHtml)
15181550
/* {{{ Returns true if this node represents text (no markup) */
15191551
PHP_METHOD(tidyNode, isText)
15201552
{
1521-
TIDY_FETCH_ONLY_OBJECT;
1553+
TIDY_FETCH_VALID_NODE;
15221554

15231555
RETURN_BOOL(tidyNodeGetType(obj->node) == TidyNode_Text);
15241556
}
@@ -1527,7 +1559,7 @@ PHP_METHOD(tidyNode, isText)
15271559
/* {{{ Returns true if this node is JSTE */
15281560
PHP_METHOD(tidyNode, isJste)
15291561
{
1530-
TIDY_FETCH_ONLY_OBJECT;
1562+
TIDY_FETCH_VALID_NODE;
15311563

15321564
RETURN_BOOL(tidyNodeGetType(obj->node) == TidyNode_Jste);
15331565
}
@@ -1536,7 +1568,7 @@ PHP_METHOD(tidyNode, isJste)
15361568
/* {{{ Returns true if this node is ASP */
15371569
PHP_METHOD(tidyNode, isAsp)
15381570
{
1539-
TIDY_FETCH_ONLY_OBJECT;
1571+
TIDY_FETCH_VALID_NODE;
15401572

15411573
RETURN_BOOL(tidyNodeGetType(obj->node) == TidyNode_Asp);
15421574
}
@@ -1545,7 +1577,7 @@ PHP_METHOD(tidyNode, isAsp)
15451577
/* {{{ Returns true if this node is PHP */
15461578
PHP_METHOD(tidyNode, isPhp)
15471579
{
1548-
TIDY_FETCH_ONLY_OBJECT;
1580+
TIDY_FETCH_VALID_NODE;
15491581

15501582
RETURN_BOOL(tidyNodeGetType(obj->node) == TidyNode_Php);
15511583
}
@@ -1554,7 +1586,7 @@ PHP_METHOD(tidyNode, isPhp)
15541586
/* {{{ Returns the parent node if available or NULL */
15551587
PHP_METHOD(tidyNode, getParent)
15561588
{
1557-
TIDY_FETCH_ONLY_OBJECT;
1589+
TIDY_FETCH_VALID_NODE;
15581590

15591591
TidyNode parent_node = tidyGetParent(obj->node);
15601592
if (parent_node) {
@@ -1565,7 +1597,7 @@ PHP_METHOD(tidyNode, getParent)
15651597

15661598
PHP_METHOD(tidyNode, getPreviousSibling)
15671599
{
1568-
TIDY_FETCH_ONLY_OBJECT;
1600+
TIDY_FETCH_VALID_NODE;
15691601

15701602
TidyNode previous_node = tidyGetPrev(obj->node);
15711603
if (previous_node) {
@@ -1575,7 +1607,7 @@ PHP_METHOD(tidyNode, getPreviousSibling)
15751607

15761608
PHP_METHOD(tidyNode, getNextSibling)
15771609
{
1578-
TIDY_FETCH_ONLY_OBJECT;
1610+
TIDY_FETCH_VALID_NODE;
15791611

15801612
TidyNode next_node = tidyGetNext(obj->node);
15811613
if (next_node) {

0 commit comments

Comments
 (0)