From 423320d4093f7f60d91475bcd35a66d32094ce2b Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Thu, 2 Nov 2023 01:35:21 +0100
Subject: [PATCH 1/2] Fix validation logic of php:function() callbacks in dom
 and xsl

Two issues:
- Assumed that at least 1 argument (function name) was provided.
- Incorrect error path for the non-callable case.
---
 ext/dom/tests/php_function_edge_cases.phpt | 27 +++++++++++++
 ext/dom/xpath.c                            |  8 +++-
 ext/xsl/tests/php_function_edge_cases.phpt | 45 ++++++++++++++++++++++
 ext/xsl/xsltprocessor.c                    |  5 +++
 4 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 ext/dom/tests/php_function_edge_cases.phpt
 create mode 100644 ext/xsl/tests/php_function_edge_cases.phpt

diff --git a/ext/dom/tests/php_function_edge_cases.phpt b/ext/dom/tests/php_function_edge_cases.phpt
new file mode 100644
index 0000000000000..1091b50ce19bd
--- /dev/null
+++ b/ext/dom/tests/php_function_edge_cases.phpt
@@ -0,0 +1,27 @@
+--TEST--
+php:function() edge cases
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$doc = new DOMDocument();
+$doc->loadHTML('<a href="https://php.net">hello</a>');
+$xpath = new DOMXpath($doc);
+$xpath->registerNamespace("php", "http://php.net/xpath");
+$xpath->registerPHPFunctions();
+try {
+    $xpath->query("//a[php:function(3)]");
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $xpath->query("//a[php:function()]");
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Handler name must be a string
+Function name must be passed as the first argument
diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index 62e11f6b99bfb..7dda6bb8b72d1 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -70,6 +70,11 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 		return;
 	}
 
+	if (UNEXPECTED(nargs == 0)) {
+		zend_throw_error(NULL, "Function name must be passed as the first argument");
+		return;
+	}
+
 	fci.param_count = nargs - 1;
 	if (fci.param_count > 0) {
 		fci.params = safe_emalloc(fci.param_count, sizeof(zval), 0);
@@ -132,7 +137,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 	if (obj->stringval == NULL) {
 		zend_type_error("Handler name must be a string");
 		xmlXPathFreeObject(obj);
-		goto cleanup;
+		goto cleanup_no_callable;
 	}
 	ZVAL_STRING(&fci.function_name, (char *) obj->stringval);
 	xmlXPathFreeObject(obj);
@@ -176,6 +181,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 	}
 cleanup:
 	zend_string_release_ex(callable, 0);
+cleanup_no_callable:
 	zval_ptr_dtor_nogc(&fci.function_name);
 	if (fci.param_count > 0) {
 		for (i = 0; i < nargs - 1; i++) {
diff --git a/ext/xsl/tests/php_function_edge_cases.phpt b/ext/xsl/tests/php_function_edge_cases.phpt
new file mode 100644
index 0000000000000..23a06b111bb50
--- /dev/null
+++ b/ext/xsl/tests/php_function_edge_cases.phpt
@@ -0,0 +1,45 @@
+--TEST--
+php:function() edge cases
+--EXTENSIONS--
+xsl
+--FILE--
+<?php
+
+function test($input) {
+    $xsl = new DomDocument();
+    $xsl->loadXML('<?xml version="1.0" encoding="iso-8859-1" ?>
+    <xsl:stylesheet version="1.0"
+    xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+    xmlns:php="http://php.net/xsl">
+    <xsl:template match="/">
+    <xsl:value-of select="php:function(' . $input . ')" />
+    </xsl:template>
+    </xsl:stylesheet>');
+
+    $inputdom = new DomDocument();
+    $inputdom->loadXML('<?xml version="1.0" encoding="iso-8859-1" ?>
+    <today></today>');
+
+    $proc = new XsltProcessor();
+    $proc->registerPhpFunctions();
+    $xsl = $proc->importStylesheet($xsl);
+    try {
+        $proc->transformToDoc($inputdom);
+    } catch (Exception $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
+
+try {
+    test("");
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+test("3");
+
+?>
+--EXPECTF--
+Function name must be passed as the first argument
+
+Warning: XSLTProcessor::transformToDoc(): Handler name must be a string in %s on line %d
diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c
index 0ddef864f050f..5de4b809ff9a5 100644
--- a/ext/xsl/xsltprocessor.c
+++ b/ext/xsl/xsltprocessor.c
@@ -141,6 +141,11 @@ static void xsl_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, int t
 		return;
 	}
 
+	if (UNEXPECTED(nargs == 0)) {
+		zend_throw_error(NULL, "Function name must be passed as the first argument");
+		return;
+	}
+
 	fci.param_count = nargs - 1;
 	if (fci.param_count > 0) {
 		args = safe_emalloc(fci.param_count, sizeof(zval), 0);

From 8bee0756143008e13fb03dd9e26e4ceddcfa4642 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Thu, 2 Nov 2023 13:27:38 +0100
Subject: [PATCH 2/2] Fix misplaced label

---
 ext/dom/xpath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index 7dda6bb8b72d1..af85e898ba44c 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -181,8 +181,8 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 	}
 cleanup:
 	zend_string_release_ex(callable, 0);
-cleanup_no_callable:
 	zval_ptr_dtor_nogc(&fci.function_name);
+cleanup_no_callable:
 	if (fci.param_count > 0) {
 		for (i = 0; i < nargs - 1; i++) {
 			zval_ptr_dtor(&fci.params[i]);