Skip to content

Commit 3221a07

Browse files
committed
ext/dom: fix UAF when setting an attribute colliding by local name.
Fix #22447 xmlHasProp() matches an attribute by local name only, ignoring its namespace, whereas xmlAddChild()/xmlAddPrevSibling() dedup an incoming no-namespace attribute via xmlHasNsProp(..., NULL), which matches only attributes with no namespace. When both a no-namespace and a namespaced attribute share a local name, the pre-insertion check unlinked the wrong (namespaced) attribute, leaving libxml to free the still-wrapped no-namespace duplicate and producing a use-after-free at request shutdown. Use xmlHasNsProp(..., NULL) so the pre-insertion check matches libxml's internal duplicate detection, as advised by @nwellnhof. Close GH-22452
1 parent 5614918 commit 3221a07

4 files changed

Lines changed: 33 additions & 3 deletions

File tree

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ PHP NEWS
88
- DOM:
99
. Fixed bug GH-22825 (DOMElement::setAttribute() fails silently when the DTD
1010
declares a default value for the attribute). (iliaal)
11+
. Fixed bug GH-22447 (UAF at dom_objects_free_storage when setting an
12+
attribute node that collides by local name with a namespaced
13+
attribute). (David Carlier)
1114

1215
- Sockets:
1316
. Fixed various memory related issues in ext/sockets. (David Carlier)

ext/dom/element.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,8 @@ static void dom_element_set_attribute_node_common(INTERNAL_FUNCTION_PARAMETERS,
726726
nsp = attrp->ns;
727727
if (use_ns && nsp != NULL) {
728728
existattrp = xmlHasNsProp(nodep, attrp->name, nsp->href);
729+
} else if (nsp == NULL) {
730+
existattrp = xmlHasNsProp(nodep, attrp->name, NULL);
729731
} else {
730732
existattrp = xmlHasProp(nodep, attrp->name);
731733
}

ext/dom/node.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
965965
xmlAttrPtr lastattr;
966966

967967
if (child->ns == NULL)
968-
lastattr = xmlHasProp(refp->parent, child->name);
968+
lastattr = xmlHasNsProp(refp->parent, child->name, NULL);
969969
else
970970
lastattr = xmlHasNsProp(refp->parent, child->name, child->ns->href);
971971
if (lastattr != NULL && lastattr->type != XML_ATTRIBUTE_DECL) {
@@ -1012,7 +1012,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
10121012
xmlAttrPtr lastattr;
10131013

10141014
if (child->ns == NULL)
1015-
lastattr = xmlHasProp(parentp, child->name);
1015+
lastattr = xmlHasNsProp(parentp, child->name, NULL);
10161016
else
10171017
lastattr = xmlHasNsProp(parentp, child->name, child->ns->href);
10181018
if (lastattr != NULL && lastattr->type != XML_ATTRIBUTE_DECL) {
@@ -1374,7 +1374,7 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern,
13741374
xmlAttrPtr lastattr;
13751375

13761376
if (child->ns == NULL)
1377-
lastattr = xmlHasProp(nodep, child->name);
1377+
lastattr = xmlHasNsProp(nodep, child->name, NULL);
13781378
else
13791379
lastattr = xmlHasNsProp(nodep, child->name, child->ns->href);
13801380
if (lastattr != NULL && lastattr->type != XML_ATTRIBUTE_DECL) {

ext/dom/tests/gh22447.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
GH-22447 (UAF at dom_objects_free_storage when setAttributeNode collides with a namespaced attribute of the same local name)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$dom = Dom\HTMLDocument::createEmpty();
8+
9+
$attribute1 = $dom->createAttribute("my-attribute");
10+
$container = $dom->appendChild($dom->createElement("container"));
11+
$attribute2 = $dom->createAttribute("my-attribute");
12+
$attribute4 = $dom->createAttributeNS("urn:a", "my-attribute");
13+
14+
$container->setAttributeNode($attribute1);
15+
$container->setAttributeNode($attribute4);
16+
17+
var_dump($container->setAttributeNode($attribute2) === $attribute1);
18+
var_dump($container->setAttributeNode($attribute1) === $attribute2);
19+
20+
echo $dom->saveXml($container), PHP_EOL;
21+
?>
22+
--EXPECT--
23+
bool(true)
24+
bool(true)
25+
<container xmlns="http://www.w3.org/1999/xhtml" xmlns:ns1="urn:a" ns1:my-attribute="" my-attribute=""></container>

0 commit comments

Comments
 (0)