Bug #80927 [Ver->Csd]: Removing documentElement after creating attribute node: possible use-after-free
Edit report at https://bugs.php.net/bug.php?id=80927&edit=1
ID: 80927
Updated by: git@php.net
Reported by: jking at jkingweb dot ca
Summary: Removing documentElement after creating attribute
node: possible use-after-free
-Status: Verified
+Status: Closed
Type: Bug
Package: DOM XML related
Operating System: any
PHP Version: 7.4
Block user comment: N
Private report: N
New Comment:
Automatic comment on behalf of nielsdos
Revision: https://github.com/php/php-src/commit/bb092ab4c6fa36b56c89216f3a127fa763940bf0
Log: Fix #80927: Removing documentElement after creating attribute node: possible use-after-free
Previous Comments:
------------------------------------------------------------------------
[2021-04-06 13:41:17] cmb@php.net
> I don't believe that passage of the specification is relevant.
Ah, right!
And yes, this looks like a refcounting issue. The documentElement
holds a pointer to the XML_NAMESPACE_DECL node (in its nsDef
member), and if it is freed, that XML_NAMESPACE_DECL node is freed
as well, resulting in the use-after-free scenario you reported.
------------------------------------------------------------------------
[2021-04-03 15:50:17] jking at jkingweb dot ca
I don't believe that passage of the specification is relevant. That error would relevant in a
scenario with the following document:
| <root>
| <a/>
| <b/>
| </root>
And when trying to do this:
$a_element->removeChild($b_element);
Because <b> is not a child of <a>.
In this case $document->documentElement is indeed a child of $document and per spec there is no
problem: the operation ought to be allowed and work.
The problem appears to be an implementation quirk of libxml, or of how PHP uses libxml. In order to
support namespaces for attribute nodes with no ownerElement it needs a root element in the document,
and if you remove this root element (and its last reference becomes garbage-collected) the namespace
information is lost.
------------------------------------------------------------------------
[2021-04-02 14:23:39] cmb@php.net
I can confirm this for PHP-7.4 as well.
It looks related to bug #66783 (which has recently been fixed);
only in this case it is about removing instead of inserting
DOMDocuments.
The standard says[1]:
| If childâs parent is not parent, then throw a "NotFoundError"
| DOMException.
[1] <https://dom.spec.whatwg.org/#concept-node-pre-remove>
------------------------------------------------------------------------
[2021-04-02 13:39:10] jking at jkingweb dot ca
Description:
------------
See test script. While it is somewhat contrived, it is a situation I ran into setting up unit tests.
When removing the document element out from under an attribute node, the namespaceURI and prefix
properties of the DOMAttr will contain garbage bytes instead of the specified values. As the prefix
property is writeable it may be possible to corrupt memory this way, though I have not confirmed
this.
Test script:
---------------
$d = new \DOMDocument();
$d->appendChild($d->createElement("html"));
$a = $d->createAttributeNS("fake_ns", "test:test");
$d->removeChild($d->documentElement);
echo $a->namespaceURI;
echo $a->prefix;
Expected result:
----------------
Obviously I would expect the namespaceURI and prefix properties not to be corrupted. Given the
apparent underlying limitations of libxml I might expect an exception to be thrown when trying to
remove the document element.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=80927&edit=1
Thread (5 messages)