From 20ac42e1b065e23376e7ea548995636369809a7d Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 19 Aug 2023 23:54:53 +0200
Subject: [PATCH] Fix memory leak when setting an invalid DOMDocument encoding

Because the failure path did not release the string, there was a memory
leak.
As the only valid types for this function are IS_NULL and IS_STRING, we
and IS_NULL is always rejected in practice, solve the issue by not using
a function that increments the refcount in the first place.

Closes GH-12002.
---
 NEWS                       |  3 +++
 ext/dom/document.c         | 19 ++++++++++++-------
 ext/dom/tests/gh12002.phpt | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 7 deletions(-)
 create mode 100644 ext/dom/tests/gh12002.phpt

diff --git a/NEWS b/NEWS
index d3e73e95c97..a4d3499a583 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,9 @@ PHP                                                                        NEWS
 - Core:
   . Fixed bug GH-11937 (Constant ASTs containing objects). (ilutov)
 
+- DOM:
+  . Fix memory leak when setting an invalid DOMDocument encoding. (nielsdos)
+
 - Iconv:
   . Fixed build for NetBSD which still uses the old iconv signature.
     (David Carlier)
diff --git a/ext/dom/document.c b/ext/dom/document.c
index ea6daafeb30..64da4f051be 100644
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -139,7 +139,6 @@ int dom_document_encoding_read(dom_object *obj, zval *retval)
 zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
 {
 	xmlDoc *docp = (xmlDocPtr) dom_object_get_node(obj);
-	zend_string *str;
 	xmlCharEncodingHandlerPtr handler;
 
 	if (docp == NULL) {
@@ -147,11 +146,15 @@ zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
 		return FAILURE;
 	}
 
-	str = zval_try_get_string(newval);
-	if (UNEXPECTED(!str)) {
-		return FAILURE;
+	/* Typed property, can only be IS_STRING or IS_NULL. */
+	ZEND_ASSERT(Z_TYPE_P(newval) == IS_STRING || Z_TYPE_P(newval) == IS_NULL);
+
+	if (Z_TYPE_P(newval) == IS_NULL) {
+		goto invalid_encoding;
 	}
 
+	zend_string *str = Z_STR_P(newval);
+
 	handler = xmlFindCharEncodingHandler(ZSTR_VAL(str));
 
 	if (handler != NULL) {
@@ -161,12 +164,14 @@ zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
 		}
 		docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str));
 	} else {
-		zend_value_error("Invalid document encoding");
-		return FAILURE;
+		goto invalid_encoding;
 	}
 
-	zend_string_release_ex(str, 0);
 	return SUCCESS;
+
+invalid_encoding:
+	zend_value_error("Invalid document encoding");
+	return FAILURE;
 }
 
 /* }}} */
diff --git a/ext/dom/tests/gh12002.phpt b/ext/dom/tests/gh12002.phpt
new file mode 100644
index 00000000000..7d1ae944646
--- /dev/null
+++ b/ext/dom/tests/gh12002.phpt
@@ -0,0 +1,38 @@
+--TEST--
+GH-12002 (DOMDocument::encoding memory leak with invalid encoding)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+function make_nonconst(string $x) {
+    // Defeat SCCP, even with inlining
+    return str_repeat($x, random_int(1, 1));
+}
+
+$dom = new DOMDocument();
+$dom->encoding = make_nonconst('utf-8');
+var_dump($dom->encoding);
+try {
+    $dom->encoding = make_nonconst('foobar');
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+var_dump($dom->encoding);
+$dom->encoding = make_nonconst('utf-16le');
+var_dump($dom->encoding);
+try {
+    $dom->encoding = NULL;
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+var_dump($dom->encoding);
+
+?>
+--EXPECT--
+string(5) "utf-8"
+Invalid document encoding
+string(5) "utf-8"
+string(8) "utf-16le"
+Invalid document encoding
+string(8) "utf-16le"