php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #80927 Removing documentElement after creating attribute node: possible use-after-free
Submitted: 2021-04-02 13:39 UTC Modified: 2021-04-06 13:41 UTC
From: jking at jkingweb dot ca Assigned:
Status: Closed Package: DOM XML related
PHP Version: 7.4 OS: any
Private report: No CVE-ID: None
View Developer Edit
 [2021-04-02 13:39 UTC] 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.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-04-02 14:23 UTC] cmb@php.net
-Status: Open +Status: Verified -PHP Version: 8.0.3 +PHP Version: 7.4 -Assigned To: +Assigned To: cmb
 [2021-04-02 14:23 UTC] 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-03 15:50 UTC] 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-06 13:41 UTC] cmb@php.net
-Assigned To: cmb +Assigned To:
 [2021-04-06 13:41 UTC] 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.
 [2023-08-12 16:49 UTC] git@php.net 
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
 [2023-08-12 16:49 UTC] git@php.net
-Status: Verified +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 11:01:29 2024 UTC