Fix bug #79451: Using DOMDocument->replaceChild on doctype causes double free

NathanFreeman · Girgias · commit 1d4300d87076 · 2022-08-19T12:46:23.000+01:00
Closes GH-9201
diff --git a/NEWS b/NEWS
@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? 2022, PHP 8.0.24
 
+- DOM:
+  . Fixed bug  #79451 (Using DOMDocument->replaceChild on doctype causes
+     double free) (NathanFreeman)
+
 - Streams:
   . Fixed bug GH-9316 ($http_response_header is wrong for long status line).
     (cmb, timwolla)
diff --git a/ext/dom/node.c b/ext/dom/node.c
@@ -20,6 +20,7 @@
 #endif
 
 #include "php.h"
+
 #if defined(HAVE_LIBXML) && defined(HAVE_DOM)
 #include "php_dom.h"
 
@@ -1001,6 +1002,7 @@ PHP_METHOD(DOMNode, replaceChild)
 	xmlNodePtr children, newchild, oldchild, nodep;
 	dom_object *intern, *newchildobj, *oldchildobj;
 	int foundoldchild = 0, stricterror;
+	bool replacedoctype = false;
 
 	int ret;
 
@@ -1063,13 +1065,21 @@ PHP_METHOD(DOMNode, replaceChild)
 				dom_reconcile_ns(nodep->doc, newchild);
 			}
 		} else if (oldchild != newchild) {
+			xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
+			replacedoctype = (intSubset == (xmlDtd *) oldchild);
+
 			if (newchild->doc == NULL && nodep->doc != NULL) {
 				xmlSetTreeDoc(newchild, nodep->doc);
 				newchildobj->document = intern->document;
 				php_libxml_increment_doc_ref((php_libxml_node_object *)newchildobj, NULL);
 			}
+
 			xmlReplaceNode(oldchild, newchild);
 			dom_reconcile_ns(nodep->doc, newchild);
+
+			if (replacedoctype) {
+				nodep->doc->intSubset = (xmlDtd *) newchild;
+			}
 		}
 		DOM_RET_OBJ(oldchild, &ret, intern);
 		return;
@@ -1668,7 +1678,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 		buf = xmlAllocOutputBuffer(NULL);
 	}
 
-    if (buf != NULL) {
+	if (buf != NULL) {
 		ret = xmlC14NDocSaveTo(docp, nodeset, exclusive, inclusive_ns_prefixes,
 			with_comments, buf);
 	}
@@ -1683,9 +1693,9 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 		xmlXPathFreeContext(ctxp);
 	}
 
-    if (buf == NULL || ret < 0) {
-        RETVAL_FALSE;
-    } else {
+	if (buf == NULL || ret < 0) {
+		RETVAL_FALSE;
+	} else {
 		if (mode == 0) {
 #ifdef LIBXML2_NEW_BUFFER
 			ret = xmlOutputBufferGetSize(buf);
@@ -1702,7 +1712,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 				RETVAL_EMPTY_STRING();
 			}
 		}
-    }
+	}
 
 	if (buf) {
 		int bytes;
diff --git a/ext/dom/tests/bug79451.phpt b/ext/dom/tests/bug79451.phpt
@@ -0,0 +1,20 @@
+--TEST--
+Bug #79451 (Using DOMDocument->replaceChild on doctype causes double free)
+--SKIPIF--
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+$dom = new \DOMDocument();
+$dom->loadHTML("<!DOCTYPE html><p>hello</p>");
+$impl = new \DOMImplementation();
+$dt = $impl->createDocumentType("html_replace", "", "");
+$dom->replaceChild($dt, $dom->doctype);
+
+var_dump($dom->doctype->name);
+echo $dom->saveXML();
+?>
+--EXPECTF--
+string(12) "html_replace"
+<?xml version="1.0" standalone="yes"?>
+<!DOCTYPE html_replace>
+<html><body><p>hello</p></body></html>
