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

go to bug id or search bugs for

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: Verified Package: DOM XML related
PHP Version: 7.4.4 OS: Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Have you experienced this issue?
Rate the importance of this bug to you:

 [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

Add a Patch

Pull Requests

Add a Pull Request

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>
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved. 		Last updated: Mon Oct 26 05:01:24 2020 UTC