php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79451 Using DOMDocument->replaceChild on doctype causes double free (PHP 7.x)
Submitted: 2020-04-05 09:20 UTC Modified: 2020-04-07 09:12 UTC
From: giovanni at giacobbi dot net Assigned:
Status: Closed Package: DOM XML related
PHP Version: 7.4.4 OS: Linux
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: giovanni at giacobbi dot net
New email:
PHP Version: OS:

 

 [2020-04-05 09:20 UTC] giovanni at giacobbi dot net
Description:
------------
While attempting to change a loaded DOM document's DOCTYPE I ran into a double free crash, this happens after creating a new DOMDocumentType object using DOMImplementation and attempting to use replaceChild on the DOMDocument.

Side note: *please* can you provide us with a clear and official documented way to change a loaded document doctype? something that we can do between loadHTML() and saveHTML() to change the doctype..thx.


Test script:
---------------
<?php
$dom = new \DOMDocument();
$dom->loadHTML("<!DOCTYPE html><p>hello</p>");
$impl = new \DOMImplementation();
$dt = $impl->createDocumentType("html", "", "");
$dom->replaceChild($dt, $dom->doctype);


Expected result:
----------------
DOCTYPE changed in memory representation

Actual result:
--------------
free(): double free detected in tcache 2
Aborted (core dumped)

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-04-05 10:29 UTC] requinix@php.net
-Status: Open +Status: Verified
 [2020-04-05 10:29 UTC] requinix@php.net
PHP 7.4.4:

==278== Invalid free() / delete / delete[] / realloc()
==278==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==278==    by 0x5BEA125: xmlFreeDoc (in /usr/lib/x86_64-linux-gnu/libxml2.so.2.9.4)
==278==    by 0x2C9793: php_libxml_decrement_doc_ref (libxml.c:1268)
==278==    by 0x3A91F9: dom_objects_free_storage (php_dom.c:1047)
==278==    by 0x7E6902: zend_objects_store_del (zend_objects_API.c:193)
==278==    by 0x78A74F: rc_dtor_func (zend_variables.c:57)
==278==    by 0x78A6D2: i_zval_ptr_dtor (zend_variables.h:44)
==278==    by 0x78A8E4: zval_ptr_dtor (zend_variables.c:84)
==278==    by 0x7A5CD1: _zend_hash_del_el_ex (zend_hash.c:1305)
==278==    by 0x7A5DB1: _zend_hash_del_el (zend_hash.c:1328)
==278==    by 0x7A7C31: zend_hash_reverse_apply (zend_hash.c:1899)
==278==    by 0x77349F: shutdown_destructors (zend_execute_API.c:245)
==278==  Address 0xf2b24b0 is 0 bytes inside a block of size 128 free'd
==278==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==278==    by 0x2C636B: php_libxml_node_free (libxml.c:217)
==278==    by 0x2C992A: php_libxml_node_free_resource (libxml.c:1314)
==278==    by 0x2C99A0: php_libxml_node_decrement_resource (libxml.c:1332)
==278==    by 0x3A91DF: dom_objects_free_storage (php_dom.c:1044)
==278==    by 0x7E6902: zend_objects_store_del (zend_objects_API.c:193)
==278==    by 0x78A74F: rc_dtor_func (zend_variables.c:57)
==278==    by 0x7F1B79: i_zval_ptr_dtor (zend_variables.h:44)
==278==    by 0x802DD4: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1637)
==278==    by 0x867D9B: execute_ex (zend_vm_execute.h:53817)
==278==    by 0x86BE8F: zend_execute (zend_vm_execute.h:57913)
==278==    by 0x78EDB8: zend_execute_scripts (zend.c:1665)
==278==  Block was alloc'd at
==278==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==278==    by 0x5BE7F6A: xmlCreateIntSubset (in /usr/lib/x86_64-linux-gnu/libxml2.so.2.9.4)
==278==    by 0x5CC930E: xmlSAX2InternalSubset (in /usr/lib/x86_64-linux-gnu/libxml2.so.2.9.4)
==278==    by 0x5BB7EE2: ??? (in /usr/lib/x86_64-linux-gnu/libxml2.so.2.9.4)
==278==    by 0x5C1838B: ??? (in /usr/lib/x86_64-linux-gnu/libxml2.so.2.9.4)
==278==    by 0x3AF1C9: dom_load_html (document.c:2075)
==278==    by 0x3AF3A3: zim_domdocument_loadHTML (document.c:2126)
==278==    by 0x802CBE: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1618)
==278==    by 0x867D9B: execute_ex (zend_vm_execute.h:53817)
==278==    by 0x86BE8F: zend_execute (zend_vm_execute.h:57913)
==278==    by 0x78EDB8: zend_execute_scripts (zend.c:1665)
==278==    by 0x6F0177: php_execute_script (main.c:2617)

> Side note: *please* can you provide us with a clear and official documented way to change a
> loaded document doctype?
Us? No. This is DOM. We don't control how it works, we just expose the libxml API, and they don't control how it works either, they just implement the DOM spec.

As far as I can tell with some quick searching, createDocumentType + replaceChild is the "correct" approach. If I try to saveHTML using your code I get some sort of corrupted doctype, so there may be a bug in libxml. A workaround could be to remove the doctype entirely and build your own as a string to prepend to the output.
 [2020-04-07 09:12 UTC] cmb@php.net
The problem here is that the document's intSubset is not properly
updated, and so after freeing the old DTD, a dangling pointer
remains, which leads to the UAF.  According to item 11 of the
libxml2 FAQ[1], updating the intSubset is supposed to be done by
clients.  So basically we need special casing for XML_DTD_NODEs
whenever we insert or replace nodes. 

[1] <http://xmlsoft.org/FAQ.html>
 [2022-08-19 11:55 UTC] git@php.net
Automatic comment on behalf of NathanFreeman (author) and Girgias (committer)
Revision: https://github.com/php/php-src/commit/1d4300d87076410c7af784a8034fccd2badb0204
Log: Fix bug #79451: Using DOMDocument-&gt;replaceChild on doctype causes double free
 [2022-08-19 11:55 UTC] git@php.net
-Status: Verified +Status: Closed
 [2022-08-19 16:16 UTC] git@php.net
Automatic comment on behalf of NathanFreeman (author) and cmb69 (committer)
Revision: https://github.com/php/php-src/commit/6027d441c1d650cad0c74e5d973a782f4b9c7516
Log: Fix #79451: DOMDocument-&gt;replaceChild on doctype causes double free
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 13:01:29 2024 UTC