diff --git a/NEWS b/NEWS index 119d59b8906..44aac918667 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,7 @@ PHP NEWS - DOM: . Fix registerNodeClass with abstract class crashing. (nielsdos) . Add missing NULL pointer error check. (icy17) + . Fix validation logic of php:function() callbacks. (nielsdos) - Fiber: . Fixed bug GH-11121 (ReflectionFiber segfault). (danog, trowski, bwoebi) @@ -60,6 +61,7 @@ PHP NEWS - XSL: . Add missing module dependency. (nielsdos) + . Fix validation logic of php:function() callbacks. (nielsdos) 26 Oct 2023, PHP 8.1.25 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 00000000000..1091b50ce19 --- /dev/null +++ b/ext/dom/tests/php_function_edge_cases.phpt @@ -0,0 +1,27 @@ +--TEST-- +php:function() edge cases +--EXTENSIONS-- +dom +--FILE-- +loadHTML('hello'); +$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 62e11f6b99b..73ceb493627 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -70,12 +70,17 @@ 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); } /* Reverse order to pop values off ctxt stack */ - for (i = nargs - 2; i >= 0; i--) { + for (i = fci.param_count - 1; i >= 0; i--) { obj = valuePop(ctxt); switch (obj->type) { case XPATH_STRING: @@ -128,11 +133,12 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, fci.size = sizeof(fci); + /* Last element of the stack is the function name */ obj = valuePop(ctxt); 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); @@ -177,6 +183,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, cleanup: zend_string_release_ex(callable, 0); 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]); 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 00000000000..23a06b111bb --- /dev/null +++ b/ext/xsl/tests/php_function_edge_cases.phpt @@ -0,0 +1,45 @@ +--TEST-- +php:function() edge cases +--EXTENSIONS-- +xsl +--FILE-- +loadXML(' + + + + + '); + + $inputdom = new DomDocument(); + $inputdom->loadXML(' + '); + + $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 0ddef864f05..22d56c41fae 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -141,12 +141,17 @@ 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); } /* Reverse order to pop values off ctxt stack */ - for (i = nargs - 2; i >= 0; i--) { + for (i = fci.param_count - 1; i >= 0; i--) { obj = valuePop(ctxt); if (obj == NULL) { ZVAL_NULL(&args[i]); @@ -221,7 +226,7 @@ static void xsl_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, int t fci.params = NULL; } - + /* Last element of the stack is the function name */ obj = valuePop(ctxt); if (obj == NULL || obj->stringval == NULL) { php_error_docref(NULL, E_WARNING, "Handler name must be a string");