From 40e667280bcd5d6618e30ed405000a42f4d4e601 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Tue, 20 May 2025 20:36:37 +0200
Subject: [PATCH] Fix GH-18597: Heap-buffer-overflow in zend_alloc.c when
 assigning string with UTF-8 bytes

xmlSave() also can flush in some cases. When the encoding is not
available this can fail for short inputs, resulting in an empty string
which is interned but then wrongly tagged by RETURN_NEW_STR.
Fix this by checking the error condition and switching to RETURN_STR for
defense-in-depth.

This issue also exists on 8.3, but does not crash; however, due to the
different API usage internally I cannot easily fix it on 8.3. There it
gives a partial output.

Closes GH-18606.
---
 NEWS                             |  3 +++
 ext/dom/inner_html_mixin.c       |  2 +-
 ext/dom/xml_document.c           |  4 ++--
 ext/libxml/libxml.c              |  2 +-
 ext/simplexml/simplexml.c        |  3 ++-
 ext/simplexml/tests/gh18597.phpt | 17 +++++++++++++++++
 6 files changed, 26 insertions(+), 5 deletions(-)
 create mode 100644 ext/simplexml/tests/gh18597.phpt

diff --git a/NEWS b/NEWS
index 8ce2b87f1c8..2829db92cfc 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.4.9
 
+- SimpleXML:
+  . Fixed bug GH-18597 (Heap-buffer-overflow in zend_alloc.c when assigning
+    string with UTF-8 bytes). (nielsdos)
 
 06 Jun 2025, PHP 8.4.8
 
diff --git a/ext/dom/inner_html_mixin.c b/ext/dom/inner_html_mixin.c
index e72b205bf46..0af47e2cf01 100644
--- a/ext/dom/inner_html_mixin.c
+++ b/ext/dom/inner_html_mixin.c
@@ -98,7 +98,7 @@ zend_result dom_element_inner_html_read(dom_object *obj, zval *retval)
 				status |= xmlOutputBufferFlush(out);
 				status |= xmlOutputBufferClose(out);
 			}
-			(void) xmlSaveClose(ctxt);
+			status |= xmlSaveClose(ctxt);
 			xmlCharEncCloseFunc(handler);
 		}
 		if (UNEXPECTED(status < 0)) {
diff --git a/ext/dom/xml_document.c b/ext/dom/xml_document.c
index 2bd3d908d70..4d941de0f06 100644
--- a/ext/dom/xml_document.c
+++ b/ext/dom/xml_document.c
@@ -282,7 +282,7 @@ static zend_string *php_new_dom_dump_node_to_str_ex(xmlNodePtr node, int options
 		} else {
 			xmlCharEncCloseFunc(handler);
 		}
-		(void) xmlSaveClose(ctxt);
+		status |= xmlSaveClose(ctxt);
 	}
 
 	if (UNEXPECTED(status < 0)) {
@@ -319,7 +319,7 @@ zend_long php_new_dom_dump_node_to_file(const char *filename, xmlDocPtr doc, xml
 	if (EXPECTED(ctxt != NULL)) {
 		status = dom_xml_serialize(ctxt, out, node, format, false, get_private_data_from_node(node));
 		status |= xmlOutputBufferFlush(out);
-		(void) xmlSaveClose(ctxt);
+		status |= xmlSaveClose(ctxt);
 	}
 
 	size_t offset = php_stream_tell(stream);
diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c
index 5ad67d12449..c637d2cebf6 100644
--- a/ext/libxml/libxml.c
+++ b/ext/libxml/libxml.c
@@ -1519,7 +1519,7 @@ static zend_string *php_libxml_default_dump_doc_to_str(xmlDocPtr doc, int option
 	}
 
 	long status = xmlSaveDoc(ctxt, doc);
-	(void) xmlSaveClose(ctxt);
+	status |= xmlSaveClose(ctxt);
 	if (status < 0) {
 		smart_str_free_ex(&str, false);
 		return NULL;
diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
index 28923c4cb39..619f627e853 100644
--- a/ext/simplexml/simplexml.c
+++ b/ext/simplexml/simplexml.c
@@ -1404,7 +1404,8 @@ PHP_METHOD(SimpleXMLElement, asXML)
 	if (!result) {
 		RETURN_FALSE;
 	} else {
-		RETURN_NEW_STR(result);
+		/* Defense-in-depth: don't use the NEW variant in case somehow an empty string gets returned */
+		RETURN_STR(result);
 	}
 }
 /* }}} */
diff --git a/ext/simplexml/tests/gh18597.phpt b/ext/simplexml/tests/gh18597.phpt
new file mode 100644
index 00000000000..e9176bf7ae0
--- /dev/null
+++ b/ext/simplexml/tests/gh18597.phpt
@@ -0,0 +1,17 @@
+--TEST--
+GH-18597 (Heap-buffer-overflow in zend_alloc.c when assigning string with UTF-8 bytes)
+--EXTENSIONS--
+simplexml
+--FILE--
+<?php
+$sx1 = new SimpleXMLElement("<root />");
+$sx1->node[0] = 'node1';
+$node = $sx1->node[0];
+
+$node[0] = '��c';
+
+$sx1->asXML(); // Depends on the available system encodings whether this fails or not, point is, it should not crash
+echo "Done\n";
+?>
+--EXPECT--
+Done