From 1c85b06aeeea89b428a3ff7568006cb754a2ced6 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Thu, 18 Jul 2024 15:17:42 +0200
Subject: [PATCH] Fix trampoline leak in xpath callables

---
 .../tests/registerPhpFunctionNS_errors.phpt   | 19 +++++++++++++++++++
 ext/dom/xpath.c                               |  7 +++++--
 .../tests/registerPHPFunctionNS_errors.phpt   | 19 +++++++++++++++++++
 ext/xsl/xsltprocessor.c                       |  7 +++++--
 4 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/ext/dom/tests/registerPhpFunctionNS_errors.phpt b/ext/dom/tests/registerPhpFunctionNS_errors.phpt
index 985376daee914..5098d8f7a84f4 100644
--- a/ext/dom/tests/registerPhpFunctionNS_errors.phpt
+++ b/ext/dom/tests/registerPhpFunctionNS_errors.phpt
@@ -5,6 +5,11 @@ dom
 --FILE--
 <?php
 
+class TrampolineClass {
+    public function __call($name, $args) {
+    }
+}
+
 $doc = new DOMDocument();
 $doc->loadHTML('<a href="https://PHP.net">hello</a>');
 
@@ -16,6 +21,18 @@ try {
     echo $e->getMessage(), "\n";
 }
 
+try {
+    $xpath->registerPhpFunctionNS('http://php.net/xpath', 'test', [new TrampolineClass, 'test']);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    $xpath->registerPhpFunctionNS('urn:foo', '$$$', [new TrampolineClass, 'test']);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+
 try {
     $xpath->registerPhpFunctionNS('urn:foo', 'x:a', strtolower(...));
 } catch (ValueError $e) {
@@ -37,6 +54,8 @@ try {
 ?>
 --EXPECT--
 DOMXPath::registerPhpFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xpath" because it is reserved by PHP
+DOMXPath::registerPhpFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xpath" because it is reserved by PHP
+DOMXPath::registerPhpFunctionNS(): Argument #2 ($name) must be a valid callback name
 DOMXPath::registerPhpFunctionNS(): Argument #2 ($name) must be a valid callback name
 DOMXPath::registerPhpFunctionNS(): Argument #2 ($name) must not contain any null bytes
 DOMXPath::registerPhpFunctionNS(): Argument #1 ($namespaceURI) must not contain any null bytes
diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index 831a11be62ea0..19e05ce2c50fe 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -462,11 +462,12 @@ PHP_METHOD(DOMXPath, registerPhpFunctionNS)
 	ZEND_PARSE_PARAMETERS_END();
 
 	if (zend_string_equals_literal(namespace, "http://php.net/xpath")) {
+		zend_release_fcall_info_cache(&fcc);
 		zend_argument_value_error(1, "must not be \"http://php.net/xpath\" because it is reserved by PHP");
 		RETURN_THROWS();
 	}
 
-	php_dom_xpath_callbacks_update_single_method_handler(
+	if (php_dom_xpath_callbacks_update_single_method_handler(
 		&intern->xpath_callbacks,
 		intern->dom.ptr,
 		namespace,
@@ -474,7 +475,9 @@ PHP_METHOD(DOMXPath, registerPhpFunctionNS)
 		&fcc,
 		PHP_DOM_XPATH_CALLBACK_NAME_VALIDATE_NCNAME,
 		dom_xpath_register_func_in_ctx
-	);
+	) != SUCCESS) {
+		zend_release_fcall_info_cache(&fcc);
+	}
 }
 
 /* {{{ */
diff --git a/ext/xsl/tests/registerPHPFunctionNS_errors.phpt b/ext/xsl/tests/registerPHPFunctionNS_errors.phpt
index 272e6d00adb72..d8ccd52f5b344 100644
--- a/ext/xsl/tests/registerPHPFunctionNS_errors.phpt
+++ b/ext/xsl/tests/registerPHPFunctionNS_errors.phpt
@@ -5,6 +5,11 @@ xsl
 --FILE--
 <?php
 
+class TrampolineClass {
+    public function __call($name, $args) {
+    }
+}
+
 require __DIR__ . '/xpath_callables.inc';
 
 try {
@@ -13,6 +18,18 @@ try {
     echo $e->getMessage(), "\n";
 }
 
+try {
+    createProcessor([])->registerPhpFunctionNS('http://php.net/xsl', 'test', [new TrampolineClass, 'test']);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    createProcessor([])->registerPhpFunctionNS('urn:foo', '$$$', [new TrampolineClass, 'test']);
+} catch (ValueError $e) {
+    echo $e->getMessage(), "\n";
+}
+
 try {
     createProcessor([])->registerPhpFunctionNS('urn:foo', 'x:a', strtolower(...));
 } catch (ValueError $e) {
@@ -34,6 +51,8 @@ try {
 ?>
 --EXPECT--
 XSLTProcessor::registerPHPFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xsl" because it is reserved by PHP
+XSLTProcessor::registerPHPFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xsl" because it is reserved by PHP
+XSLTProcessor::registerPHPFunctionNS(): Argument #2 ($name) must be a valid callback name
 XSLTProcessor::registerPHPFunctionNS(): Argument #2 ($name) must be a valid callback name
 XSLTProcessor::registerPHPFunctionNS(): Argument #2 ($name) must not contain any null bytes
 XSLTProcessor::registerPHPFunctionNS(): Argument #1 ($namespaceURI) must not contain any null bytes
diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c
index 3ad528aa53870..3dbb1ad16c99d 100644
--- a/ext/xsl/xsltprocessor.c
+++ b/ext/xsl/xsltprocessor.c
@@ -707,11 +707,12 @@ PHP_METHOD(XSLTProcessor, registerPHPFunctionNS)
 	ZEND_PARSE_PARAMETERS_END();
 
 	if (zend_string_equals_literal(namespace, "http://php.net/xsl")) {
+		zend_release_fcall_info_cache(&fcc);
 		zend_argument_value_error(1, "must not be \"http://php.net/xsl\" because it is reserved by PHP");
 		RETURN_THROWS();
 	}
 
-	php_dom_xpath_callbacks_update_single_method_handler(
+	if (php_dom_xpath_callbacks_update_single_method_handler(
 		&intern->xpath_callbacks,
 		NULL,
 		namespace,
@@ -719,7 +720,9 @@ PHP_METHOD(XSLTProcessor, registerPHPFunctionNS)
 		&fcc,
 		PHP_DOM_XPATH_CALLBACK_NAME_VALIDATE_NCNAME,
 		NULL
-	);
+	) != SUCCESS) {
+		zend_release_fcall_info_cache(&fcc);
+	}
 }
 
 /* {{{ */