Skip to content

Commit a59103f

Browse files
committed
Fix trampoline leak in xpath callables
Closes GH-15009.
1 parent 5471f3d commit a59103f

File tree

5 files changed

+54
-4
lines changed

5 files changed

+54
-4
lines changed

NEWS

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ PHP NEWS
55
- Core:
66
. Fix GH-14978 (The xmlreader extension phpize build). (Peter Kokot)
77

8+
- DOM:
9+
. Fix trampoline leak in xpath callables. (nielsdos)
10+
811
- OpenSSL:
912
. Bumped minimum required OpenSSL version to 1.1.0. (cmb)
1013

@@ -16,6 +19,9 @@ PHP NEWS
1619
- Standard:
1720
. Fix references in request_parse_body() options array. (nielsdos)
1821

22+
- XSL:
23+
. Fix trampoline leak in xpath callables. (nielsdos)
24+
1925
18 Jul 2024, PHP 8.4.0alpha2
2026

2127
- Core:

ext/dom/tests/registerPhpFunctionNS_errors.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ dom
55
--FILE--
66
<?php
77

8+
class TrampolineClass {
9+
public function __call($name, $args) {
10+
}
11+
}
12+
813
$doc = new DOMDocument();
914
$doc->loadHTML('<a href="https://PHP.net">hello</a>');
1015

@@ -16,6 +21,18 @@ try {
1621
echo $e->getMessage(), "\n";
1722
}
1823

24+
try {
25+
$xpath->registerPhpFunctionNS('http://php.net/xpath', 'test', [new TrampolineClass, 'test']);
26+
} catch (ValueError $e) {
27+
echo $e->getMessage(), "\n";
28+
}
29+
30+
try {
31+
$xpath->registerPhpFunctionNS('urn:foo', '$$$', [new TrampolineClass, 'test']);
32+
} catch (ValueError $e) {
33+
echo $e->getMessage(), "\n";
34+
}
35+
1936
try {
2037
$xpath->registerPhpFunctionNS('urn:foo', 'x:a', strtolower(...));
2138
} catch (ValueError $e) {
@@ -37,6 +54,8 @@ try {
3754
?>
3855
--EXPECT--
3956
DOMXPath::registerPhpFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xpath" because it is reserved by PHP
57+
DOMXPath::registerPhpFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xpath" because it is reserved by PHP
58+
DOMXPath::registerPhpFunctionNS(): Argument #2 ($name) must be a valid callback name
4059
DOMXPath::registerPhpFunctionNS(): Argument #2 ($name) must be a valid callback name
4160
DOMXPath::registerPhpFunctionNS(): Argument #2 ($name) must not contain any null bytes
4261
DOMXPath::registerPhpFunctionNS(): Argument #1 ($namespaceURI) must not contain any null bytes

ext/dom/xpath.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,19 +462,22 @@ PHP_METHOD(DOMXPath, registerPhpFunctionNS)
462462
ZEND_PARSE_PARAMETERS_END();
463463

464464
if (zend_string_equals_literal(namespace, "http://php.net/xpath")) {
465+
zend_release_fcall_info_cache(&fcc);
465466
zend_argument_value_error(1, "must not be \"http://php.net/xpath\" because it is reserved by PHP");
466467
RETURN_THROWS();
467468
}
468469

469-
php_dom_xpath_callbacks_update_single_method_handler(
470+
if (php_dom_xpath_callbacks_update_single_method_handler(
470471
&intern->xpath_callbacks,
471472
intern->dom.ptr,
472473
namespace,
473474
name,
474475
&fcc,
475476
PHP_DOM_XPATH_CALLBACK_NAME_VALIDATE_NCNAME,
476477
dom_xpath_register_func_in_ctx
477-
);
478+
) != SUCCESS) {
479+
zend_release_fcall_info_cache(&fcc);
480+
}
478481
}
479482

480483
/* {{{ */

ext/xsl/tests/registerPHPFunctionNS_errors.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ xsl
55
--FILE--
66
<?php
77

8+
class TrampolineClass {
9+
public function __call($name, $args) {
10+
}
11+
}
12+
813
require __DIR__ . '/xpath_callables.inc';
914

1015
try {
@@ -13,6 +18,18 @@ try {
1318
echo $e->getMessage(), "\n";
1419
}
1520

21+
try {
22+
createProcessor([])->registerPhpFunctionNS('http://php.net/xsl', 'test', [new TrampolineClass, 'test']);
23+
} catch (ValueError $e) {
24+
echo $e->getMessage(), "\n";
25+
}
26+
27+
try {
28+
createProcessor([])->registerPhpFunctionNS('urn:foo', '$$$', [new TrampolineClass, 'test']);
29+
} catch (ValueError $e) {
30+
echo $e->getMessage(), "\n";
31+
}
32+
1633
try {
1734
createProcessor([])->registerPhpFunctionNS('urn:foo', 'x:a', strtolower(...));
1835
} catch (ValueError $e) {
@@ -34,6 +51,8 @@ try {
3451
?>
3552
--EXPECT--
3653
XSLTProcessor::registerPHPFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xsl" because it is reserved by PHP
54+
XSLTProcessor::registerPHPFunctionNS(): Argument #1 ($namespaceURI) must not be "http://php.net/xsl" because it is reserved by PHP
55+
XSLTProcessor::registerPHPFunctionNS(): Argument #2 ($name) must be a valid callback name
3756
XSLTProcessor::registerPHPFunctionNS(): Argument #2 ($name) must be a valid callback name
3857
XSLTProcessor::registerPHPFunctionNS(): Argument #2 ($name) must not contain any null bytes
3958
XSLTProcessor::registerPHPFunctionNS(): Argument #1 ($namespaceURI) must not contain any null bytes

ext/xsl/xsltprocessor.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -707,19 +707,22 @@ PHP_METHOD(XSLTProcessor, registerPHPFunctionNS)
707707
ZEND_PARSE_PARAMETERS_END();
708708

709709
if (zend_string_equals_literal(namespace, "http://php.net/xsl")) {
710+
zend_release_fcall_info_cache(&fcc);
710711
zend_argument_value_error(1, "must not be \"http://php.net/xsl\" because it is reserved by PHP");
711712
RETURN_THROWS();
712713
}
713714

714-
php_dom_xpath_callbacks_update_single_method_handler(
715+
if (php_dom_xpath_callbacks_update_single_method_handler(
715716
&intern->xpath_callbacks,
716717
NULL,
717718
namespace,
718719
name,
719720
&fcc,
720721
PHP_DOM_XPATH_CALLBACK_NAME_VALIDATE_NCNAME,
721722
NULL
722-
);
723+
) != SUCCESS) {
724+
zend_release_fcall_info_cache(&fcc);
725+
}
723726
}
724727

725728
/* {{{ */

0 commit comments

Comments
 (0)