Skip to content

Commit b9b231f

Browse files
committed
Merge branch 'PHP-8.6'
* PHP-8.6: Fix UAF in reflected attribute write paths (#25120)
2 parents e92d9c4 + bbb21f0 commit b9b231f

5 files changed

Lines changed: 77 additions & 8 deletions

File tree

‎ext/dom/element.c‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ static xmlAttrPtr dom_element_reflected_attribute_write(dom_object *obj, zval *n
155155
/* Typed property, so it is a string already */
156156
ZEND_ASSERT(Z_TYPE_P(newval) == IS_STRING);
157157
php_libxml_invalidate_node_list_cache(obj->document);
158-
return xmlSetNsProp(nodep, NULL, (const xmlChar *) name, (const xmlChar *) Z_STRVAL_P(newval));
158+
return dom_set_attribute_value(nodep, NULL, (const xmlChar *) name, (const xmlChar *) Z_STRVAL_P(newval));
159159
}
160160

161161
/* {{{ className string
@@ -1060,11 +1060,7 @@ static void dom_set_attribute_ns_modern(dom_object *intern, xmlNodePtr elemp, ze
10601060
if (errorcode == 0) {
10611061
php_dom_libxml_ns_mapper *ns_mapper = php_dom_get_ns_mapper(intern);
10621062
xmlNsPtr ns = php_dom_libxml_ns_mapper_get_ns_raw_prefix_string(ns_mapper, prefix, xmlStrlen(prefix), uri);
1063-
xmlNodePtr existing = (xmlNodePtr) xmlHasNsProp(elemp, localname, ns == NULL ? NULL : ns->href);
1064-
if (existing != NULL && existing->type != XML_ATTRIBUTE_DECL) {
1065-
node_list_unlink(existing->children);
1066-
}
1067-
xmlAttrPtr attr = xmlSetNsProp(elemp, ns, localname, BAD_CAST value);
1063+
xmlAttrPtr attr = dom_set_attribute_value(elemp, ns, localname, BAD_CAST value);
10681064
if (UNEXPECTED(attr == NULL)) {
10691065
php_dom_throw_error(INVALID_STATE_ERR, /* strict */ true);
10701066
} else {

‎ext/dom/php_dom.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2350,6 +2350,15 @@ void dom_remove_all_children(xmlNodePtr nodep)
23502350
}
23512351
}
23522352

2353+
xmlAttrPtr dom_set_attribute_value(xmlNodePtr element, xmlNsPtr ns, const xmlChar *name, const xmlChar *value)
2354+
{
2355+
xmlAttrPtr existing = xmlHasNsProp(element, name, ns == NULL ? NULL : ns->href);
2356+
if (existing != NULL && existing->type != XML_ATTRIBUTE_DECL) {
2357+
node_list_unlink(existing->children);
2358+
}
2359+
return xmlSetNsProp(element, ns, name, value);
2360+
}
2361+
23532362
void php_dom_get_content_into_zval(const xmlNode *nodep, zval *return_value, bool null_on_failure)
23542363
{
23552364
ZEND_ASSERT(nodep != NULL);

‎ext/dom/php_dom.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ zend_object *dom_xpath_objects_new(zend_class_entry *class_type);
112112
#endif
113113
bool dom_get_strict_error(php_libxml_ref_obj *document);
114114
void node_list_unlink(xmlNodePtr node);
115+
xmlAttrPtr dom_set_attribute_value(xmlNodePtr element, xmlNsPtr ns, const xmlChar *name, const xmlChar *value);
115116
int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, int name_len);
116117
xmlNsPtr dom_get_ns(xmlNodePtr node, char *uri, int *errorcode, char *prefix);
117118
xmlNsPtr dom_get_ns_unchecked(xmlNodePtr nodep, char *uri, char *prefix);
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
--TEST--
2+
Reflected attribute writes keep attribute children with live wrappers
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function check(string $label, $doc, Closure $write): void {
9+
$element = $doc->documentElement;
10+
$text = $element->getAttributeNode('class')->firstChild;
11+
12+
$write($element);
13+
14+
echo $label, ": detached: ";
15+
var_dump($text->parentNode === null);
16+
echo $label, ": text: ";
17+
var_dump($text->textContent);
18+
echo $label, ": new value: ";
19+
var_dump($element->getAttribute('class'));
20+
}
21+
22+
$doc = new DOMDocument();
23+
$doc->loadXML('<root class="old"/>');
24+
check('legacy className', $doc, static fn ($element) => $element->className = 'new');
25+
26+
$doc = Dom\XMLDocument::createFromString('<root class="old"/>');
27+
check('modern className', $doc, static fn ($element) => $element->className = 'new');
28+
29+
$doc = Dom\XMLDocument::createFromString('<root class="old"/>');
30+
check('classList value', $doc, static fn ($element) => $element->classList->value = 'new');
31+
32+
$doc = Dom\XMLDocument::createFromString('<root class="old"/>');
33+
check('classList update', $doc, static fn ($element) => $element->classList->add('new'));
34+
35+
$doc = Dom\XMLDocument::createFromString('<root class="old"/>');
36+
$element = $doc->documentElement;
37+
$element->id = 'old';
38+
$text = $element->getAttributeNode('id')->firstChild;
39+
$element->id = 'new';
40+
echo "id: detached: ";
41+
var_dump($text->parentNode === null);
42+
echo "id: text: ";
43+
var_dump($text->textContent);
44+
echo "id: new value: ";
45+
var_dump($element->id);
46+
47+
?>
48+
--EXPECT--
49+
legacy className: detached: bool(true)
50+
legacy className: text: string(3) "old"
51+
legacy className: new value: string(3) "new"
52+
modern className: detached: bool(true)
53+
modern className: text: string(3) "old"
54+
modern className: new value: string(3) "new"
55+
classList value: detached: bool(true)
56+
classList value: text: string(3) "old"
57+
classList value: new value: string(3) "new"
58+
classList update: detached: bool(true)
59+
classList update: text: string(3) "old"
60+
classList update: new value: string(7) "old new"
61+
id: detached: bool(true)
62+
id: text: string(3) "old"
63+
id: new value: string(3) "new"

‎ext/dom/token_list.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ static void dom_token_list_update(dom_token_list_object *intern)
192192
/* 2. Set an attribute value for the associated element using associated attribute’s local name and the result of
193193
* running the ordered set serializer for token set. */
194194
char *value = dom_ordered_set_serializer(token_set);
195-
xmlSetNsProp(dom_token_list_get_element(intern), NULL, BAD_CAST "class", BAD_CAST value);
195+
dom_set_attribute_value(dom_token_list_get_element(intern), NULL, BAD_CAST "class", BAD_CAST value);
196196
efree(intern->cached_string);
197197
intern->cached_string = value;
198198
}
@@ -432,7 +432,7 @@ zend_result dom_token_list_value_write(dom_object *obj, zval *newval)
432432
return FAILURE;
433433
}
434434
php_libxml_invalidate_node_list_cache(intern->dom.document);
435-
xmlSetNsProp(dom_token_list_get_element(intern), NULL, BAD_CAST "class", BAD_CAST Z_STRVAL_P(newval));
435+
dom_set_attribute_value(dom_token_list_get_element(intern), NULL, BAD_CAST "class", BAD_CAST Z_STRVAL_P(newval));
436436
/* Note: we don't update the set here, the set is always lazily updated for performance reasons. */
437437
return SUCCESS;
438438
}

0 commit comments

Comments
 (0)