From 64b47c52af8daacccbeede00696224ca1d0d964b 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.
---
 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 +++++++++++++++++
 5 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 ext/simplexml/tests/gh18597.phpt

diff --git a/ext/dom/inner_html_mixin.c b/ext/dom/inner_html_mixin.c
index e72b205bf4628..0af47e2cf019f 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 2bd3d908d7093..4d941de0f0686 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 5ad67d1244987..c637d2cebf6a4 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 28923c4cb3925..619f627e8532a 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 0000000000000..e9176bf7ae041
--- /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