From 9ce73f2fc95f8ee08c7f5d2ea90442db6bb28720 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 9 Nov 2026 16:24:51 -0500 Subject: [PATCH 1/3] ext/xsl: Fix use-after-free on a nested transform of the same processor php_xsl_apply_stylesheet() keeps the transformation's document reference in intern->doc and the callback node wrappers in the processor's node list, so a second transformation on the same processor, started from a php:function() callback or from main code while a Fiber is suspended inside one, freed that state under the running transformation. Throw an Error for such a call, as importStylesheet() already does, and restore transform_depth when a callback bails out so that the processor remains usable in shutdown functions. --- NEWS | 4 + UPGRADING | 7 ++ .../tests/xsltprocessor_nested_transform.phpt | 100 ++++++++++++++++++ ext/xsl/xsltprocessor.c | 18 +++- 4 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 ext/xsl/tests/xsltprocessor_nested_transform.phpt diff --git a/NEWS b/NEWS index 5a865955281d..f32d6714a140 100644 --- a/NEWS +++ b/NEWS @@ -204,6 +204,10 @@ PHP NEWS . Fixed xml_get_current_byte_index() returning negative values for byte indexes beyond 2 GiB on Windows x64. (Marc Bennewitz) +- XSL: + . Fixed a use-after-free when a transformation was started on a processor + that is already transforming. (Ilia Alshanetsky) + - Zlib: . Fixed inflate_init() dropping the preset dictionary for raw streams with a non-default window. (Ilia Alshanetsky) diff --git a/UPGRADING b/UPGRADING index a4df23352639..e0554f2014a7 100644 --- a/UPGRADING +++ b/UPGRADING @@ -64,6 +64,13 @@ PHP 8.7 UPGRADE NOTES to 16 by default. Set the stream context option max_filter_count to change this. +- XSL: + . XSLTProcessor::transformToDoc(), XSLTProcessor::transformToUri() and + XSLTProcessor::transformToXml() now throw an Error when called while the + same processor is already transforming, for example from a php:function() + callback or from a suspended Fiber. Use a separate XSLTProcessor instance + instead. + ======================================== 2. New Features ======================================== diff --git a/ext/xsl/tests/xsltprocessor_nested_transform.phpt b/ext/xsl/tests/xsltprocessor_nested_transform.phpt new file mode 100644 index 000000000000..cef56cc67925 --- /dev/null +++ b/ext/xsl/tests/xsltprocessor_nested_transform.phpt @@ -0,0 +1,100 @@ +--TEST-- +XSLTProcessor refuses a nested transformation on the same processor +--EXTENSIONS-- +dom +xsl +--FILE-- +transformToXml($doc)); + } catch (Error $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; + } + try { + var_dump($proc->transformToDoc($doc)); + } catch (Error $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; + } + try { + var_dump($proc->transformToUri($doc, $GLOBALS['uri'])); + } catch (Error $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; + } + return 'v'; +} + +function suspend($nodes) { + Fiber::suspend(); + return 'f'; +} + +function text($nodes) { + return $nodes[0]->textContent; +} + +$doc = new DOMDocument(); +$doc->loadXML('123'); + +$xsl = new DOMDocument(); +$xsl->loadXML(<< + + + [] + +XML); + +$uri = __DIR__ . '/xsltprocessor_nested_transform.xml'; + +$proc = new XSLTProcessor(); +$proc->registerPHPFunctions(); +$proc->importStylesheet($xsl); + +var_dump($proc->transformToXml($doc)); + +$proc->setParameter('', 'cb', 'suspend'); +$fiber = new Fiber(fn () => $proc->transformToXml($doc)); +$fiber->start(); +try { + var_dump($proc->transformToXml($doc)); +} catch (Error $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} +while (!$fiber->isTerminated()) { + $fiber->resume(); +} +var_dump($fiber->getReturn()); + +$proc->setParameter('', 'cb', 'text'); +var_dump($proc->transformToXml($doc)); +var_dump($proc->transformToDoc($doc)->textContent); + +?> +--CLEAN-- + +--EXPECT-- +Error: Cannot call XSLTProcessor::transformToXml() while a transformation is in progress +Error: Cannot call XSLTProcessor::transformToDoc() while a transformation is in progress +Error: Cannot call XSLTProcessor::transformToUri() while a transformation is in progress +string(35) " +[va][vb][vc] +" +Error: Cannot call XSLTProcessor::transformToXml() while a transformation is in progress +string(35) " +[fa][fb][fc] +" +string(35) " +[1a][2b][3c] +" +string(12) "[1a][2b][3c]" diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index 3f6a7b6aa923..f66ae07a57b3 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -303,6 +303,7 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl FILE *f; int secPrefsError = 0; xsltSecurityPrefsPtr secPrefs = NULL; + bool bailout = false; node = php_libxml_import_node(docp); @@ -323,6 +324,13 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl return NULL; } + if (UNEXPECTED(intern->transform_depth > 0)) { + zend_string *name = get_active_function_or_method_name(); + zend_throw_error(NULL, "Cannot call %s() while a transformation is in progress", ZSTR_VAL(name)); + zend_string_release(name); + return NULL; + } + intern->transform_depth++; if (intern->profiling) { @@ -409,7 +417,11 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl if (secPrefsError == 1) { php_error_docref(NULL, E_WARNING, "Can't set libxslt security properties, not doing transformation for security reasons"); } else { - newdocp = xsltApplyStylesheetUser(style, doc, /* params (handled manually) */ NULL, /* output */ NULL, f, ctxt); + zend_try { + newdocp = xsltApplyStylesheetUser(style, doc, NULL, NULL, f, ctxt); + } zend_catch { + bailout = true; + } zend_end_try(); } out: @@ -430,6 +442,10 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl intern->transform_depth--; + if (bailout) { + zend_bailout(); + } + return newdocp; } From e4fff9575fcab9343dbb45c345e6da67d4000d82 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 9 Nov 2026 19:28:48 -0500 Subject: [PATCH 2/3] ext/dom: Fix use-after-free when an XPath callback modifies the document A php:function() callback runs while libxml2 builds an XPath node-set, so modifying the tree or dropping the last reference to a detached subtree freed nodes the evaluation still referenced. While DOMXPath or Dom\XPath evaluates with PHP functions registered, the tree of the evaluated document, the tree of every node a callback returned and the detached subtrees of the context node and of callback arguments are read-only: modifying them throws a DOMException with NO_MODIFICATION_ALLOWED_ERR or, through SimpleXML, an Error, and the evaluation keeps those subtrees alive until it ends. New nodes and document properties stay modifiable. --- NEWS | 5 + UPGRADING | 11 ++ ext/dom/attr.c | 4 + ext/dom/cdatasection.c | 4 + ext/dom/characterdata.c | 15 +++ ext/dom/comment.c | 4 + ext/dom/document.c | 16 +++ ext/dom/documentfragment.c | 8 ++ ext/dom/domimplementation.c | 6 + ext/dom/element.c | 64 +++++++++ ext/dom/entityreference.c | 4 + ext/dom/html_document.c | 6 + ext/dom/node.c | 24 ++++ ext/dom/parentnode/tree.c | 46 +++++++ ext/dom/php_dom.c | 14 ++ ext/dom/php_dom.h | 10 ++ ext/dom/processinginstruction.c | 4 + .../xpath_php_function_detached_tree.phpt | 123 ++++++++++++++++++ .../xpath_php_function_modifies_document.phpt | 120 +++++++++++++++++ ...hp_function_modifies_document_bailout.phpt | 38 ++++++ ..._php_function_modifies_document_fiber.phpt | 44 +++++++ ...hp_function_modifies_foreign_document.phpt | 44 +++++++ ...th_php_function_removes_argument_node.phpt | 10 +- ..._function_simplexml_modifies_document.phpt | 50 +++++++ ext/dom/text.c | 8 ++ ext/dom/token_list.c | 15 +++ ext/dom/xpath.c | 63 +++++++-- ext/dom/xpath_callbacks.c | 97 +++++++++++++- ext/dom/xpath_callbacks.h | 10 ++ ext/libxml/libxml.c | 16 +++ ext/libxml/php_libxml.h | 12 ++ ext/simplexml/simplexml.c | 29 ++++- 32 files changed, 909 insertions(+), 15 deletions(-) create mode 100644 ext/dom/tests/xpath_php_function_detached_tree.phpt create mode 100644 ext/dom/tests/xpath_php_function_modifies_document.phpt create mode 100644 ext/dom/tests/xpath_php_function_modifies_document_bailout.phpt create mode 100644 ext/dom/tests/xpath_php_function_modifies_document_fiber.phpt create mode 100644 ext/dom/tests/xpath_php_function_modifies_foreign_document.phpt create mode 100644 ext/dom/tests/xpath_php_function_simplexml_modifies_document.phpt diff --git a/NEWS b/NEWS index f32d6714a140..734f986001af 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,11 @@ PHP NEWS passing an array of CURLFile or CURLStringFile objects to CURLOPT_POSTFIELDS. (Sjoerd Langkemper) +- DOM: + . Fixed use-after-free when a php:function() callback of DOMXPath or + Dom\XPath modifies nodes that are still being evaluated; such + modifications now throw a DOMException or Error. (Ilia Alshanetsky) + - FPM: . Use recommended TYPE, HELP order for OpenMetrics metadata. (Marcel Hernandez) diff --git a/UPGRADING b/UPGRADING index e0554f2014a7..ee2184d46a27 100644 --- a/UPGRADING +++ b/UPGRADING @@ -29,6 +29,17 @@ PHP 8.7 UPGRADE NOTES . Nested arrays in CURLOPT_POSTFIELDS now throw a TypeError, instead of being silently converted to the string "Array". +- DOM: + . While DOMXPath or Dom\XPath evaluates an expression with PHP functions + registered, the nodes it can reach are read-only: the tree of the + evaluated document, the tree of every node a php:function() callback + returned, and the detached subtree of the context node or of a node + passed to a callback. Modifying such a node throws a DOMException with + code DOM_NO_MODIFICATION_ALLOWED_ERR (a warning when strictErrorChecking + is disabled), and modifying it through SimpleXML throws an Error. + Creating and modifying new nodes and setting document properties such as + formatOutput or documentURI are still allowed. + - PDO: . PDOStatement::bindColumn() now throws a ValueError when the column name is not present in the result set. It previously reported the condition diff --git a/ext/dom/attr.c b/ext/dom/attr.c index aa613a9f6d37..a0b41d10d5a5 100644 --- a/ext/dom/attr.c +++ b/ext/dom/attr.c @@ -45,6 +45,10 @@ PHP_METHOD(DOMAttr, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + intern = Z_DOMOBJ_P(ZEND_THIS); name_valid = xmlValidateName(BAD_CAST name, 0); diff --git a/ext/dom/cdatasection.c b/ext/dom/cdatasection.c index b735a93f3bee..9822c725d195 100644 --- a/ext/dom/cdatasection.c +++ b/ext/dom/cdatasection.c @@ -40,6 +40,10 @@ PHP_METHOD(DOMCdataSection, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + nodep = xmlNewCDataBlock(NULL, BAD_CAST value, value_len); if (!nodep) { diff --git a/ext/dom/characterdata.c b/ext/dom/characterdata.c index 80dce80cf2f0..39d0b6632fde 100644 --- a/ext/dom/characterdata.c +++ b/ext/dom/characterdata.c @@ -164,6 +164,9 @@ static void dom_character_data_append_data(INTERNAL_FUNCTION_PARAMETERS) } DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + return; + } xmlTextConcat(nodep, BAD_CAST arg, arg_len); } @@ -202,6 +205,10 @@ static void dom_character_data_insert_data(INTERNAL_FUNCTION_PARAMETERS, bool re DOM_GET_OBJ(node, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, node)) { + RETURN_FALSE; + } + const xmlChar *cur = php_dom_get_content_or_empty(node); length = xmlUTF8Strlen(cur); @@ -267,6 +274,10 @@ static void dom_character_data_delete_data(INTERNAL_FUNCTION_PARAMETERS, bool re DOM_GET_OBJ(node, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, node)) { + RETURN_FALSE; + } + const xmlChar *cur = php_dom_get_content_or_empty(node); length = xmlUTF8Strlen(cur); @@ -337,6 +348,10 @@ static void dom_character_data_replace_data(INTERNAL_FUNCTION_PARAMETERS, bool r DOM_GET_OBJ(node, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, node)) { + RETURN_FALSE; + } + const xmlChar *cur = php_dom_get_content_or_empty(node); length = xmlUTF8Strlen(cur); diff --git a/ext/dom/comment.c b/ext/dom/comment.c index b8ee8780d926..66cbf02cb768 100644 --- a/ext/dom/comment.c +++ b/ext/dom/comment.c @@ -40,6 +40,10 @@ PHP_METHOD(DOMComment, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + nodep = xmlNewComment(BAD_CAST value); if (!nodep) { diff --git a/ext/dom/document.c b/ext/dom/document.c index ccb2c08bc815..1b5e2470a5e8 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -1213,6 +1213,10 @@ static void dom_document_adopt_node(INTERNAL_FUNCTION_PARAMETERS, bool modern) RETURN_FALSE; } + if (dom_throw_if_node_read_only(dom_object_nodep->document, nodep)) { + RETURN_FALSE; + } + xmlDocPtr new_document; dom_object *dom_object_new_document; zval *new_document_zval = ZEND_THIS; @@ -1254,6 +1258,10 @@ PHP_METHOD(DOMDocument, normalizeDocument) DOM_GET_OBJ(docp, id, xmlDocPtr, intern); + if (dom_throw_if_node_read_only(intern->document, (xmlNodePtr) docp)) { + return; + } + php_dom_normalize_legacy((xmlNodePtr) docp); } /* }}} end dom_document_normalize_document */ @@ -1801,6 +1809,10 @@ PHP_METHOD(DOMDocument, xinclude) DOM_GET_OBJ(docp, ZEND_THIS, xmlDocPtr, intern); + if (dom_throw_if_node_read_only(intern->document, (xmlNodePtr) docp)) { + RETURN_FALSE; + } + int err = dom_perform_xinclude(docp, intern, flags); if (err) { @@ -1827,6 +1839,10 @@ PHP_METHOD(Dom_XMLDocument, xinclude) DOM_GET_OBJ(docp, ZEND_THIS, xmlDocPtr, intern); + if (dom_throw_if_node_read_only(intern->document, (xmlNodePtr) docp)) { + RETURN_THROWS(); + } + int err = dom_perform_xinclude(docp, intern, flags); if (err < 0) { diff --git a/ext/dom/documentfragment.c b/ext/dom/documentfragment.c index 01af436c69ed..ce43f701fab0 100644 --- a/ext/dom/documentfragment.c +++ b/ext/dom/documentfragment.c @@ -36,6 +36,10 @@ PHP_METHOD(DOMDocumentFragment, __construct) ZEND_PARSE_PARAMETERS_NONE(); + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + nodep = xmlNewDocFragment(NULL); if (!nodep) { @@ -74,6 +78,10 @@ PHP_METHOD(DOMDocumentFragment, appendXML) { RETURN_FALSE; } + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_FALSE; + } + if (data) { PHP_LIBXML_SANITIZE_GLOBALS(parse); err = xmlParseBalancedChunkMemory(nodep->doc, NULL, NULL, 0, BAD_CAST data, &lst); diff --git a/ext/dom/domimplementation.c b/ext/dom/domimplementation.c index e7fc2a414834..22a2cc9765b7 100644 --- a/ext/dom/domimplementation.c +++ b/ext/dom/domimplementation.c @@ -174,6 +174,9 @@ PHP_METHOD(DOMImplementation, createDocument) php_dom_throw_error(WRONG_DOCUMENT_ERR, true); RETURN_THROWS(); } + if (dom_throw_if_node_read_only(doctobj->document, (xmlNodePtr) doctype)) { + RETURN_THROWS(); + } } else { doctobj = NULL; } @@ -261,6 +264,9 @@ PHP_METHOD(Dom_Implementation, createDocument) if (dtd != NULL) { DOM_GET_OBJ(doctype, dtd, xmlDtdPtr, doctobj); + if (dom_throw_if_node_read_only(doctobj->document, (xmlNodePtr) doctype)) { + RETURN_THROWS(); + } } xmlDocPtr document = NULL; diff --git a/ext/dom/element.c b/ext/dom/element.c index bab828815a63..aacfcb13df8b 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -50,6 +50,10 @@ PHP_METHOD(DOMElement, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + name_valid = xmlValidateName(BAD_CAST name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, true); @@ -429,6 +433,10 @@ PHP_METHOD(DOMElement, setAttribute) DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_FALSE; + } + if (php_dom_follow_spec_intern(intern)) { xmlChar *name_processed = BAD_CAST name; if (php_dom_ns_is_html_and_document_is_html(nodep)) { @@ -619,6 +627,10 @@ PHP_METHOD(DOMElement, removeAttribute) DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_FALSE; + } + attrp = dom_get_attribute_or_nsdecl(intern, nodep, BAD_CAST name, name_len); if (attrp == NULL) { RETURN_FALSE; @@ -640,6 +652,10 @@ PHP_METHOD(Dom_Element, removeAttribute) DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_THROWS(); + } + attrp = dom_get_attribute_or_nsdecl(intern, nodep, BAD_CAST name, name_len); if (attrp != NULL) { dom_remove_attribute(nodep, attrp, intern->document); @@ -699,6 +715,10 @@ static void dom_element_set_attribute_node_common(INTERNAL_FUNCTION_PARAMETERS, DOM_GET_OBJ(nodep, id, xmlNodePtr, intern); DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj); + if (dom_throw_if_node_read_only(intern->document, nodep) || dom_throw_if_node_read_only(attrobj->document, (xmlNodePtr) attrp)) { + RETURN_FALSE; + } + /* ZPP Guarantees that a DOMAttr class is given, as it is converted to a xmlAttr * to pass to libxml (see http://www.xmlsoft.org/html/libxml-tree.html#xmlAttr) * if it is not of type XML_ATTRIBUTE_NODE it indicates a bug somewhere */ @@ -795,6 +815,10 @@ static void dom_element_remove_attribute_node(INTERNAL_FUNCTION_PARAMETERS, zend ZEND_ASSERT(attrp->type == XML_ATTRIBUTE_NODE); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_FALSE; + } + if (attrp->parent != nodep) { php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document)); RETURN_FALSE; @@ -1095,6 +1119,10 @@ PHP_METHOD(DOMElement, setAttributeNS) DOM_GET_OBJ(elemp, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, elemp)) { + return; + } + if (php_dom_follow_spec_intern(intern)) { dom_set_attribute_ns_modern(intern, elemp, uri, name, value); } else { @@ -1185,6 +1213,10 @@ PHP_METHOD(DOMElement, removeAttributeNS) DOM_GET_OBJ(nodep, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + return; + } + bool follow_spec = php_dom_follow_spec_intern(intern); if (follow_spec && uri_len == 0) { uri = NULL; @@ -1408,6 +1440,10 @@ PHP_METHOD(DOMElement, setIdAttribute) DOM_GET_OBJ(nodep, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + return; + } + attrp = xmlHasNsProp(nodep, BAD_CAST name, NULL); if (attrp == NULL || attrp->type == XML_ATTRIBUTE_DECL) { php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document)); @@ -1437,6 +1473,10 @@ PHP_METHOD(DOMElement, setIdAttributeNS) DOM_GET_OBJ(elemp, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, elemp)) { + return; + } + attrp = xmlHasNsProp(elemp, BAD_CAST name, BAD_CAST uri); if (attrp == NULL || attrp->type == XML_ATTRIBUTE_DECL) { php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document)); @@ -1465,6 +1505,10 @@ static void dom_element_set_id_attribute_node(INTERNAL_FUNCTION_PARAMETERS, zend DOM_GET_OBJ(nodep, id, xmlNodePtr, intern); DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + return; + } + if (attrp->parent != nodep) { php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document)); } else { @@ -1665,6 +1709,10 @@ static void dom_element_insert_adjacent_element(INTERNAL_FUNCTION_PARAMETERS, ze DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, this_intern); DOM_GET_OBJ(otherp, element_zval, xmlNodePtr, other_intern); + if (dom_throw_if_node_read_only(this_intern->document, thisp) || dom_throw_if_node_read_only(other_intern->document, otherp)) { + RETURN_NULL(); + } + xmlNodePtr result = dom_insert_adjacent(where, thisp, this_intern, otherp); if (result == NULL) { RETURN_NULL(); @@ -1741,6 +1789,10 @@ static void dom_element_insert_adjacent_text(INTERNAL_FUNCTION_PARAMETERS, zend_ DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, this_intern); + if (dom_throw_if_node_read_only(this_intern->document, thisp)) { + return; + } + if (UNEXPECTED(ZEND_SIZE_T_INT_OVFL(ZSTR_LEN(data)))) { zend_argument_value_error(2, "is too long"); RETURN_THROWS(); @@ -1809,6 +1861,10 @@ PHP_METHOD(Dom_Element, insertAdjacentHTML) DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, this_intern); + if (dom_throw_if_node_read_only(this_intern->document, thisp)) { + RETURN_THROWS(); + } + /* 1. We don't do injection sinks. */ /* 2. Let context be NULL */ @@ -1899,6 +1955,10 @@ PHP_METHOD(DOMElement, toggleAttribute) DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, thisp)) { + RETURN_FALSE; + } + /* Step 1 */ if (xmlValidateName(BAD_CAST qname, 0) != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, true); @@ -2177,6 +2237,10 @@ PHP_METHOD(Dom_Element, rename) xmlNodePtr nodep; DOM_GET_THIS_OBJ(nodep, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_THROWS(); + } + xmlChar *localname = NULL, *prefix = NULL; int errorcode = dom_validate_and_extract(namespace_uri, qualified_name, &localname, &prefix); if (UNEXPECTED(errorcode != 0)) { diff --git a/ext/dom/entityreference.c b/ext/dom/entityreference.c index a669fd2aa791..be14144bc3f3 100644 --- a/ext/dom/entityreference.c +++ b/ext/dom/entityreference.c @@ -42,6 +42,10 @@ PHP_METHOD(DOMEntityReference, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + name_valid = xmlValidateName(BAD_CAST name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, true); diff --git a/ext/dom/html_document.c b/ext/dom/html_document.c index 5f190ec7c97b..2f866f94add8 100644 --- a/ext/dom/html_document.c +++ b/ext/dom/html_document.c @@ -1547,6 +1547,9 @@ zend_result dom_html_document_body_write(dom_object *obj, zval *newval) dom_object *newval_intern = Z_DOMOBJ_P(newval); if (newval_intern->ptr != NULL) { xmlNodePtr newval_node = ((php_libxml_node_ptr *) newval_intern->ptr)->node; + if (dom_throw_if_node_read_only(obj->document, (xmlNodePtr) docp) || dom_throw_if_node_read_only(newval_intern->document, newval_node)) { + return FAILURE; + } /* Note: because this property has type HTMLElement, we know the namespace is correct. */ if (dom_accept_body_name(newval_node->name)) { /* 2. If the new value is the same as the body element, return. */ @@ -1684,6 +1687,9 @@ static void dom_string_replace_all(xmlDocPtr docp, xmlNodePtr element, zval *zv) zend_result dom_html_document_title_write(dom_object *obj, zval *newval) { DOM_PROP_NODE(xmlDocPtr, docp, obj); + if (dom_throw_if_node_read_only(obj->document, (xmlNodePtr) docp)) { + return FAILURE; + } xmlNodePtr root = xmlDocGetRootElement(docp); if (root == NULL) { diff --git a/ext/dom/node.c b/ext/dom/node.c index 88c058a6ac51..9e5cb43b86b8 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -1104,6 +1104,10 @@ static void dom_node_insert_before(INTERNAL_FUNCTION_PARAMETERS, bool modern) DOM_GET_OBJ(child, node, xmlNodePtr, childobj); + if (dom_throw_if_node_read_only(intern->document, parentp) || dom_throw_if_node_read_only(childobj->document, child)) { + RETURN_FALSE; + } + if (modern) { dom_node_insert_before_modern(return_value, ref, intern, parentp, child); } else { @@ -1226,6 +1230,10 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern) DOM_GET_OBJ(newchild, newnode, xmlNodePtr, newchildobj); DOM_GET_OBJ(oldchild, oldnode, xmlNodePtr, oldchildobj); + if (dom_throw_if_node_read_only(intern->document, nodep) || dom_throw_if_node_read_only(newchildobj->document, newchild)) { + RETURN_FALSE; + } + bool stricterror = dom_get_strict_error(intern->document); if (newchild->doc != nodep->doc && newchild->doc != NULL) { @@ -1325,6 +1333,10 @@ static void dom_node_remove_child(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry DOM_GET_OBJ(child, node, xmlNodePtr, childobj); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + RETURN_FALSE; + } + bool stricterror = dom_get_strict_error(intern->document); if (!nodep->children || child->parent != nodep) { @@ -1456,6 +1468,10 @@ PHP_METHOD(DOMNode, appendChild) DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern); DOM_GET_OBJ(child, node, xmlNodePtr, childobj); + if (dom_throw_if_node_read_only(intern->document, nodep) || dom_throw_if_node_read_only(childobj->document, child)) { + RETURN_FALSE; + } + dom_node_append_child_legacy(return_value, intern, childobj, nodep, child); } @@ -1472,6 +1488,10 @@ PHP_METHOD(Dom_Node, appendChild) DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern); DOM_GET_OBJ(child, node, xmlNodePtr, childobj); + if (dom_throw_if_node_read_only(intern->document, nodep) || dom_throw_if_node_read_only(childobj->document, child)) { + RETURN_THROWS(); + } + /* Parent check from pre-insertion validation done here: * If parent is not a Document, DocumentFragment, or Element node, then throw a "HierarchyRequestError" DOMException. */ if (php_dom_pre_insert_is_parent_invalid(nodep)) { @@ -1583,6 +1603,10 @@ PHP_METHOD(DOMNode, normalize) DOM_GET_OBJ(nodep, id, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, nodep)) { + return; + } + if (php_dom_follow_spec_intern(intern)) { php_dom_normalize_modern(nodep); } else { diff --git a/ext/dom/parentnode/tree.c b/ext/dom/parentnode/tree.c index aacc991cc735..75f385d0fb55 100644 --- a/ext/dom/parentnode/tree.c +++ b/ext/dom/parentnode/tree.c @@ -482,6 +482,24 @@ static zend_result dom_sanity_check_node_list_types(zval *nodes, uint32_t nodesc return SUCCESS; } +static bool dom_throw_if_nodes_read_only(dom_object *context, const zval *nodes, uint32_t nodesc) +{ + if (dom_throw_if_node_read_only(context->document, dom_object_get_node(context))) { + return true; + } + + for (uint32_t i = 0; i < nodesc; i++) { + if (Z_TYPE(nodes[i]) == IS_OBJECT) { + dom_object *intern = Z_DOMOBJ_P(&nodes[i]); + if (dom_throw_if_node_read_only(intern->document, dom_object_get_node(intern))) { + return true; + } + } + } + + return false; +} + static void php_dom_pre_insert_helper(xmlNodePtr insertion_point, xmlNodePtr parentNode, xmlNodePtr newchild, xmlNodePtr last) { if (!insertion_point) { @@ -609,6 +627,10 @@ void dom_parent_node_append(dom_object *context, zval *nodes, uint32_t nodesc) return; } + if (dom_throw_if_nodes_read_only(context, nodes, nodesc)) { + return; + } + xmlNode *parentNode = dom_object_get_node(context); php_libxml_invalidate_node_list_cache(context->document); @@ -637,6 +659,10 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc) return; } + if (dom_throw_if_nodes_read_only(context, nodes, nodesc)) { + return; + } + php_libxml_invalidate_node_list_cache(context->document); /* 1. Let node be the result of converting nodes into a node given nodes and this’s node document. */ @@ -656,6 +682,10 @@ void dom_parent_node_after(dom_object *context, zval *nodes, uint32_t nodesc) return; } + if (dom_throw_if_nodes_read_only(context, nodes, nodesc)) { + return; + } + xmlNode *thisp = dom_object_get_node(context); /* 1. Let parent be this’s parent. */ @@ -688,6 +718,10 @@ void dom_parent_node_before(dom_object *context, zval *nodes, uint32_t nodesc) return; } + if (dom_throw_if_nodes_read_only(context, nodes, nodesc)) { + return; + } + xmlNode *thisp = dom_object_get_node(context); /* 1. Let parent be this’s parent. */ @@ -722,6 +756,10 @@ void dom_parent_node_before(dom_object *context, zval *nodes, uint32_t nodesc) static zend_result dom_child_removal_preconditions(const xmlNode *child, const dom_object *context) { + if (dom_throw_if_node_read_only(context->document, child)) { + return FAILURE; + } + if (dom_node_is_read_only(child) || (child->parent != NULL && dom_node_is_read_only(child->parent))) { php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(context->document)); @@ -756,6 +794,10 @@ void dom_child_replace_with(dom_object *context, zval *nodes, uint32_t nodesc) return; } + if (dom_throw_if_nodes_read_only(context, nodes, nodesc)) { + return; + } + xmlNodePtr child = dom_object_get_node(context); /* 1. Let parent be this’s parent. */ @@ -805,6 +847,10 @@ void dom_parent_node_replace_children(dom_object *context, zval *nodes, uint32_t return; } + if (dom_throw_if_nodes_read_only(context, nodes, nodesc)) { + return; + } + xmlNodePtr thisp = dom_object_get_node(context); php_libxml_invalidate_node_list_cache(context->document); diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 0833d9a3c9be..c70664673ba3 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -170,6 +170,11 @@ bool dom_node_is_read_only(const xmlNode *node) { } } +ZEND_COLD void php_dom_throw_read_only_node_error(php_libxml_ref_obj *document) +{ + php_dom_throw_error_with_message(NO_MODIFICATION_ALLOWED_ERR, "Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress", dom_get_strict_error(document)); +} + bool dom_node_children_valid(const xmlNode *node) { switch (node->type) { case XML_DOCUMENT_TYPE_NODE: @@ -432,6 +437,15 @@ zval *dom_write_property(zend_object *object, zend_string *name, zval *value, vo return &EG(error_zval); } + if (UNEXPECTED(obj->document == NULL || obj->document->read_only_depth > 0) + && (instanceof_function(object->ce, dom_node_class_entry) || instanceof_function(object->ce, dom_modern_node_class_entry))) { + const xmlNode *nodep = dom_object_get_node(obj); + if (nodep != NULL && nodep->type != XML_DOCUMENT_NODE && nodep->type != XML_HTML_DOCUMENT_NODE + && dom_throw_if_node_read_only(obj->document, nodep)) { + return value; + } + } + ZEND_ASSERT(prop && ZEND_TYPE_IS_SET(prop->type)); zval tmp; ZVAL_COPY(&tmp, value); diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 24475022e614..32f77257af79 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -126,6 +126,7 @@ void php_dom_create_implementation(zval *retval, bool modern); int dom_hierarchy(xmlNodePtr parent, xmlNodePtr child); bool dom_has_feature(zend_string *feature, zend_string *version); bool dom_node_is_read_only(const xmlNode *node); +ZEND_COLD void php_dom_throw_read_only_node_error(php_libxml_ref_obj *document); bool dom_node_children_valid(const xmlNode *node); xmlNodePtr create_notation(xmlDtdPtr parent_dtd, const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID); void dom_free_notation(xmlEntityPtr entity); @@ -259,6 +260,15 @@ static zend_always_inline void php_dom_mark_cache_tag_up_to_date_from_doc_ref(ph cache_tag->modification_nr = document->cache_tag.modification_nr; } +static zend_always_inline bool dom_throw_if_node_read_only(php_libxml_ref_obj *document, const xmlNode *node) +{ + if (php_libxml_is_node_read_only(document, node)) { + php_dom_throw_read_only_node_error(document); + return true; + } + return false; +} + static zend_always_inline bool php_dom_follow_spec_node(const xmlNode *node) { ZEND_ASSERT(node != NULL); diff --git a/ext/dom/processinginstruction.c b/ext/dom/processinginstruction.c index 94e5d5b35067..75bff8594839 100644 --- a/ext/dom/processinginstruction.c +++ b/ext/dom/processinginstruction.c @@ -42,6 +42,10 @@ PHP_METHOD(DOMProcessingInstruction, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + name_valid = xmlValidateName(BAD_CAST name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, true); diff --git a/ext/dom/tests/xpath_php_function_detached_tree.phpt b/ext/dom/tests/xpath_php_function_detached_tree.phpt new file mode 100644 index 000000000000..88f7fe691b42 --- /dev/null +++ b/ext/dom/tests/xpath_php_function_detached_tree.phpt @@ -0,0 +1,123 @@ +--TEST-- +DOMXPath: detached subtrees and returned nodes stay alive while a php:function() evaluation uses them +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +function subtree() { + $tree = $GLOBALS['doc']->createElement('tree'); + foreach (['a', 'b', 'c'] as $name) { + $tree->appendChild($GLOBALS['doc']->createElement($name)); + } + return $tree; +} + +function drop() { + unset($GLOBALS['tree']); + return true; +} + +function first_of_tree() { + return $GLOBALS['tree']->firstChild; +} + +function reconstruct() { + try { + $GLOBALS['tree']->__construct('other'); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + return true; +} + +$tree = subtree(); +$context = $tree->firstChild; +foreach ($xp->query('../*[php:function("drop")]', $context) as $node) { + echo $node->nodeName, "\n"; +} + +$tree = subtree(); +foreach ($xp->query('php:function("first_of_tree")/../*[php:function("drop")]') as $node) { + echo $node->nodeName, "\n"; +} + +$tree = subtree(); +foreach ($xp->query('*[php:function("reconstruct")]', $tree) as $node) { + echo $node->nodeName, "\n"; +} + +class Unconstructed extends DOMElement { + public function __construct() {} +} + +function unconstructed() { + return new Unconstructed(); +} + +var_dump($xp->evaluate('count(php:function("unconstructed"))')); + +$docless = new DOMElement('docless', 'abc'); + +function docless() { + return $GLOBALS['docless']; +} + +function modify_docless() { + try { + $GLOBALS['docless']->textContent = 'xyz'; + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + return true; +} + +foreach ($xp->query('php:function("docless")/text()[php:function("modify_docless")]') as $node) { + var_dump($node->nodeValue); +} +$docless->textContent = 'xyz'; +var_dump($docless->textContent); + +$doctype = (new DOMImplementation())->createDocumentType('q'); + +function doctype() { + return $GLOBALS['doctype']; +} + +function attach_doctype() { + try { + (new DOMImplementation())->createDocument('', 'r', $GLOBALS['doctype']); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + return 1; +} + +var_dump($xp->evaluate('count(php:function("doctype")) + php:function("attach_doctype")')); +var_dump($doctype->ownerDocument); +?> +--EXPECT-- +a +b +c +a +b +c +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +a +b +c +float(0) +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +string(3) "abc" +string(3) "xyz" +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +float(2) +NULL diff --git a/ext/dom/tests/xpath_php_function_modifies_document.phpt b/ext/dom/tests/xpath_php_function_modifies_document.phpt new file mode 100644 index 000000000000..064a0dd2bb48 --- /dev/null +++ b/ext/dom/tests/xpath_php_function_modifies_document.phpt @@ -0,0 +1,120 @@ +--TEST-- +DOMXPath: a php:function() callback cannot modify the document that is being evaluated +--EXTENSIONS-- +dom +--FILE-- +getMessage() . " (" . $e->getCode() . ")"] = true; + } + return true; +} + +function evaluate(object $xp, string $label, Closure $mutation, string $expr = '/root/node()[php:function("cb")]') { + $GLOBALS['mutation'] = $mutation; + $GLOBALS['errors'] = []; + $res = $xp->evaluate($expr); + echo $label, ": ", count($res), " nodes\n"; + echo implode("\n", array_keys($GLOBALS['errors'])), "\n"; +} + +$doc = new DOMDocument(); +$doc->loadXML('123'); +$root = $doc->documentElement; +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +evaluate($xp, 'removeChild', function () use ($root) { + foreach (iterator_to_array($root->childNodes) as $child) { + $root->removeChild($child); + } +}); +evaluate($xp, 'replaceChild', function () use ($doc, $root) { + $root->replaceChild($doc->createElement('y'), $root->lastChild); +}); +evaluate($xp, 'textContent', function () use ($root) { + $root->textContent = 'x'; +}); +evaluate($xp, 'removeAttribute', function () use ($root) { + $root->removeAttribute('a'); +}, '/root/@*[php:function("cb")]'); +echo $doc->saveXML($root), "\n"; + +$other = new DOMDocument(); +$other->loadXML(''); +evaluate($xp, 'other document', function () use ($other) { + $other->documentElement->firstChild?->remove(); +}); +echo $other->saveXML($other->documentElement), "\n"; + +function build($nodes) { + $doc = $nodes[0]->ownerDocument; + $span = $doc->createElement('span'); + $span->setAttribute('class', 'w'); + $span->appendChild($doc->createTextNode(strtoupper($nodes[0]->textContent))); + $doc->formatOutput = true; + $doc->documentURI = 'urn:built'; + try { + $doc->documentElement->appendChild($doc->createElement('extra')); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + return $span; +} +foreach ($xp->query('php:function("build", /root/x[1])') as $node) { + echo $doc->saveXML($node), " ", $doc->documentURI, "\n"; +} +$doc->formatOutput = false; + +$doc->strictErrorChecking = false; +$GLOBALS['mutation'] = function () use ($root) { + var_dump($root->removeChild($root->firstChild)); +}; +var_dump($xp->evaluate('boolean(/root[php:function("cb")])')); +$doc->strictErrorChecking = true; + +$root->removeChild($root->firstChild); +$root->removeAttribute('a'); +echo $doc->saveXML($root), "\n"; + +$modern = Dom\XMLDocument::createFromString('12'); +$modern_xp = new Dom\XPath($modern); +$modern_xp->registerNamespace('php', 'http://php.net/xpath'); +$modern_xp->registerPhpFunctions(); +evaluate($modern_xp, 'Dom\Element::remove', function () use ($modern) { + $modern->documentElement->firstChild->remove(); +}); +evaluate($modern_xp, 'Dom\Element::$innerHTML', function () use ($modern) { + $modern->documentElement->innerHTML = ''; +}); +echo $modern->saveXml($modern->documentElement), "\n"; +?> +--EXPECTF-- +removeChild: 3 nodes +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress (7) +replaceChild: 3 nodes +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress (7) +textContent: 3 nodes +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress (7) +removeAttribute: 2 nodes +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress (7) +123 +other document: 3 nodes + + +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +1 urn:built + +Warning: DOMNode::removeChild(): Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress in %s on line %d +bool(false) +bool(true) +23 +Dom\Element::remove: 2 nodes +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress (7) +Dom\Element::$innerHTML: 2 nodes +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress (7) +12 diff --git a/ext/dom/tests/xpath_php_function_modifies_document_bailout.phpt b/ext/dom/tests/xpath_php_function_modifies_document_bailout.phpt new file mode 100644 index 000000000000..68aaa065a4d4 --- /dev/null +++ b/ext/dom/tests/xpath_php_function_modifies_document_bailout.phpt @@ -0,0 +1,38 @@ +--TEST-- +DOMXPath: a document becomes writable again after a fatal error in a php:function() callback +--EXTENSIONS-- +dom +zend_test +--SKIPIF-- + +--FILE-- +loadXML(''); +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +function cb() { + try { + $GLOBALS['doc']->documentElement->removeChild($GLOBALS['doc']->documentElement->firstChild); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + zend_trigger_bailout(); +} + +register_shutdown_function(function () use ($doc) { + $doc->documentElement->removeChild($doc->documentElement->firstChild); + echo $doc->saveXML($doc->documentElement), "\n"; +}); + +$xp->evaluate('boolean(php:function("cb"))'); +?> +--EXPECTF-- +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress + +Fatal error: Bailout in %s on line %d + diff --git a/ext/dom/tests/xpath_php_function_modifies_document_fiber.phpt b/ext/dom/tests/xpath_php_function_modifies_document_fiber.phpt new file mode 100644 index 000000000000..d4cd58e72d6d --- /dev/null +++ b/ext/dom/tests/xpath_php_function_modifies_document_fiber.phpt @@ -0,0 +1,44 @@ +--TEST-- +DOMXPath: a document stays read-only while a Fiber is suspended inside a php:function() callback +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +function suspend() { + Fiber::suspend(); + return true; +} + +$fiber = new Fiber(function () use ($xp) { + $names = []; + foreach ($xp->query('/root/*[php:function("suspend")]') as $node) { + $names[] = $node->nodeName; + } + return $names; +}); +$fiber->start(); + +try { + $doc->documentElement->removeChild($doc->documentElement->firstChild); +} catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; +} + +while (!$fiber->isTerminated()) { + $fiber->resume(); +} +echo implode(",", $fiber->getReturn()), "\n"; + +$doc->documentElement->removeChild($doc->documentElement->firstChild); +echo $doc->saveXML($doc->documentElement), "\n"; +?> +--EXPECT-- +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +a,b,c + diff --git a/ext/dom/tests/xpath_php_function_modifies_foreign_document.phpt b/ext/dom/tests/xpath_php_function_modifies_foreign_document.phpt new file mode 100644 index 000000000000..46762a3b11f9 --- /dev/null +++ b/ext/dom/tests/xpath_php_function_modifies_foreign_document.phpt @@ -0,0 +1,44 @@ +--TEST-- +DOMXPath: a php:function() callback cannot modify a document whose node another callback returned +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$other = new DOMDocument(); +$other->loadXML('123'); +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +function other_root() { + return $GLOBALS['other']->documentElement; +} + +function cb() { + $root = $GLOBALS['other']->documentElement; + try { + foreach (iterator_to_array($root->childNodes) as $child) { + $root->removeChild($child); + } + } catch (DOMException $e) { + $GLOBALS['errors'][$e::class . ": " . $e->getMessage()] = true; + } + return true; +} + +$errors = []; +$res = $xp->evaluate('php:function("other_root")/a[php:function("cb")]'); +echo "count: ", count($res), "\n"; +echo implode("\n", array_keys($errors)), "\n"; +echo $other->saveXML($other->documentElement), "\n"; + +$other->documentElement->removeChild($other->documentElement->firstChild); +echo $other->saveXML($other->documentElement), "\n"; +?> +--EXPECT-- +count: 3 +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +123 +23 diff --git a/ext/dom/tests/xpath_php_function_removes_argument_node.phpt b/ext/dom/tests/xpath_php_function_removes_argument_node.phpt index e6696617be25..9cc50c14f94c 100644 --- a/ext/dom/tests/xpath_php_function_removes_argument_node.phpt +++ b/ext/dom/tests/xpath_php_function_removes_argument_node.phpt @@ -1,5 +1,5 @@ --TEST-- -DOMXPath: a php:function callback that removes its argument node must not free it mid-evaluation +DOMXPath: a php:function callback cannot remove its argument node mid-evaluation --EXTENSIONS-- dom --FILE-- @@ -12,8 +12,10 @@ $xp->registerPhpFunctions(); function cb($nodes) { foreach ($nodes as $n) { - if ($n->parentNode) { + try { $n->parentNode->removeChild($n); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; } } return true; @@ -26,6 +28,10 @@ foreach ($res as $r) { echo "done\n"; ?> --EXPECT-- +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress string(1) "a" string(1) "a" string(1) "a" diff --git a/ext/dom/tests/xpath_php_function_simplexml_modifies_document.phpt b/ext/dom/tests/xpath_php_function_simplexml_modifies_document.phpt new file mode 100644 index 000000000000..b444b4d56a3b --- /dev/null +++ b/ext/dom/tests/xpath_php_function_simplexml_modifies_document.phpt @@ -0,0 +1,50 @@ +--TEST-- +DOMXPath: a php:function() callback cannot modify the document that is being evaluated through SimpleXML +--EXTENSIONS-- +dom +simplexml +--FILE-- +loadXML('1234'); +$xp = new DOMXPath($doc); +$xp->registerNamespace('php', 'http://php.net/xpath'); +$xp->registerPhpFunctions(); + +function cb() { + $sxe = simplexml_import_dom($GLOBALS['doc']); + $mutations = [ + 'assign' => function () use ($sxe) { $sxe->a[1] = 'z'; }, + 'unset' => function () use ($sxe) { unset($sxe->a[0]); }, + 'attribute' => function () use ($sxe) { $sxe['a'] = '2'; }, + 'addChild' => function () use ($sxe) { $sxe->addChild('c'); }, + 'addAttribute' => function () use ($sxe) { $sxe->addAttribute('d', '3'); }, + ]; + foreach ($mutations as $label => $mutation) { + try { + $mutation(); + } catch (Error $e) { + $GLOBALS['errors'][$label . ": " . $e::class . ": " . $e->getMessage()] = true; + } + } + return true; +} + +$errors = []; +var_dump(count($xp->query('(//a/node())[php:function("cb")]'))); +echo implode("\n", array_keys($errors)), "\n"; +echo $doc->saveXML($doc->documentElement), "\n"; + +$sxe = simplexml_import_dom($doc); +$sxe->a[1] = 'z'; +echo $doc->saveXML($doc->documentElement), "\n"; +?> +--EXPECT-- +int(6) +assign: Error: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +unset: Error: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +attribute: Error: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +addChild: Error: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +addAttribute: Error: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +1234 +12z diff --git a/ext/dom/text.c b/ext/dom/text.c index 357ff1969490..a807e81734eb 100644 --- a/ext/dom/text.c +++ b/ext/dom/text.c @@ -41,6 +41,10 @@ PHP_METHOD(DOMText, __construct) RETURN_THROWS(); } + if (dom_throw_if_node_read_only(Z_DOMOBJ_P(ZEND_THIS)->document, dom_object_get_node(Z_DOMOBJ_P(ZEND_THIS)))) { + return; + } + nodep = xmlNewText(BAD_CAST value); if (!nodep) { @@ -107,6 +111,10 @@ PHP_METHOD(DOMText, splitText) } DOM_GET_OBJ(node, ZEND_THIS, xmlNodePtr, intern); + if (dom_throw_if_node_read_only(intern->document, node)) { + RETURN_FALSE; + } + if (offset < 0) { zend_argument_value_error(1, "must be greater than or equal to 0"); RETURN_THROWS(); diff --git a/ext/dom/token_list.c b/ext/dom/token_list.c index 4adaf94c203d..c73770d991e4 100644 --- a/ext/dom/token_list.c +++ b/ext/dom/token_list.c @@ -431,6 +431,9 @@ zend_result dom_token_list_value_write(dom_object *obj, zval *newval) zend_value_error("Value must not contain any null bytes"); return FAILURE; } + if (dom_throw_if_node_read_only(intern->dom.document, dom_token_list_get_element(intern))) { + return FAILURE; + } php_libxml_invalidate_node_list_cache(intern->dom.document); dom_set_attribute_value(dom_token_list_get_element(intern), NULL, BAD_CAST "class", BAD_CAST Z_STRVAL_P(newval)); /* Note: we don't update the set here, the set is always lazily updated for performance reasons. */ @@ -519,6 +522,9 @@ PHP_METHOD(Dom_TokenList, add) /* 2. For each token in tokens, append token to this’s token set. */ dom_token_list_object *intern = TOKEN_LIST_GET_INTERNAL(); + if (dom_throw_if_node_read_only(intern->dom.document, dom_token_list_get_element(intern))) { + RETURN_THROWS(); + } dom_token_list_ensure_set_up_to_date(intern); HashTable *token_set = TOKEN_LIST_GET_SET(intern); for (uint32_t i = 0; i < argc; i++) { @@ -545,6 +551,9 @@ PHP_METHOD(Dom_TokenList, remove) /* 2. For each token in tokens, remove token from this’s token set. */ dom_token_list_object *intern = TOKEN_LIST_GET_INTERNAL(); + if (dom_throw_if_node_read_only(intern->dom.document, dom_token_list_get_element(intern))) { + RETURN_THROWS(); + } dom_token_list_ensure_set_up_to_date(intern); HashTable *token_set = TOKEN_LIST_GET_SET(intern); for (uint32_t i = 0; i < argc; i++) { @@ -573,6 +582,9 @@ PHP_METHOD(Dom_TokenList, toggle) /* 3. If this’s token set[token] exists, then: */ dom_token_list_object *intern = TOKEN_LIST_GET_INTERNAL(); + if (dom_throw_if_node_read_only(intern->dom.document, dom_token_list_get_element(intern))) { + RETURN_THROWS(); + } dom_token_list_ensure_set_up_to_date(intern); HashTable *token_set = TOKEN_LIST_GET_SET(intern); zval *found_token = zend_hash_find(token_set, token); @@ -619,6 +631,9 @@ PHP_METHOD(Dom_TokenList, replace) /* 3. If this’s token set does not contain token, then return false. */ dom_token_list_object *intern = TOKEN_LIST_GET_INTERNAL(); + if (dom_throw_if_node_read_only(intern->dom.document, dom_token_list_get_element(intern))) { + RETURN_THROWS(); + } dom_token_list_ensure_set_up_to_date(intern); HashTable *token_set = TOKEN_LIST_GET_SET(intern); zval *found_token = zend_hash_find(token_set, token); diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c index e3c5e48f6cd5..235a988a001e 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -273,12 +273,51 @@ PHP_METHOD(DOMXPath, registerNamespace) } /* }}} */ +static xmlXPathObjectPtr dom_xpath_eval_expression(dom_xpath_object *intern, xmlXPathContextPtr ctxp, const char *expr, dom_object *context_obj) +{ + intern->evaluation_depth++; + + if (!php_dom_xpath_callbacks_registered(&intern->xpath_callbacks)) { + xmlXPathObjectPtr xpathobjp = xmlXPathEvalExpression(BAD_CAST expr, ctxp); + intern->evaluation_depth--; + return xpathobjp; + } + + php_libxml_ref_obj *document = intern->dom.document; + xmlXPathObjectPtr xpathobjp = NULL; + bool bailout = false; + + document->read_only_depth++; + intern->xpath_callbacks.read_only_depth++; + if (context_obj != NULL) { + xmlNodePtr nodep = ctxp->node; + php_dom_xpath_callbacks_pin_tree(&intern->xpath_callbacks, nodep->type == XML_NAMESPACE_DECL ? nodep->parent : nodep, context_obj); + } + zend_try { + xpathobjp = xmlXPathEvalExpression(BAD_CAST expr, ctxp); + } zend_catch { + bailout = true; + } zend_end_try(); + intern->xpath_callbacks.read_only_depth--; + document->read_only_depth--; + intern->evaluation_depth--; + + if (bailout) { + if (intern->evaluation_depth == 0) { + php_dom_xpath_callbacks_release_read_only(&intern->xpath_callbacks); + } + zend_bailout(); + } + + return xpathobjp; +} + static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type, bool modern) /* {{{ */ { zval *context = NULL; xmlNodePtr nodep = NULL; size_t expr_len, xpath_type; - dom_object *nodeobj; + dom_object *nodeobj = NULL; char *expr; dom_xpath_object *intern = Z_XPATHOBJ_P(ZEND_THIS); @@ -336,9 +375,7 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type, bool modern) ctxp->nsNr = in_scope_ns.count; } - intern->evaluation_depth++; - xmlXPathObjectPtr xpathobjp = xmlXPathEvalExpression(BAD_CAST expr, ctxp); - intern->evaluation_depth--; + xmlXPathObjectPtr xpathobjp = dom_xpath_eval_expression(intern, ctxp, expr, context != NULL ? nodeobj : NULL); ctxp->node = old_node; ctxp->namespaces = old_namespaces; ctxp->nsNr = old_nsNr; @@ -455,27 +492,37 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type, bool modern) } /* }}} */ +static void dom_xpath_evaluate(INTERNAL_FUNCTION_PARAMETERS, int type, bool modern) +{ + php_xpath_eval(INTERNAL_FUNCTION_PARAM_PASSTHRU, type, modern); + + dom_xpath_object *intern = Z_XPATHOBJ_P(ZEND_THIS); + if (intern->evaluation_depth == 0) { + php_dom_xpath_callbacks_release_read_only(&intern->xpath_callbacks); + } +} + /* {{{ */ PHP_METHOD(DOMXPath, query) { - php_xpath_eval(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_QUERY, false); + dom_xpath_evaluate(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_QUERY, false); } PHP_METHOD(Dom_XPath, query) { - php_xpath_eval(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_QUERY, true); + dom_xpath_evaluate(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_QUERY, true); } /* }}} end dom_xpath_query */ /* {{{ */ PHP_METHOD(DOMXPath, evaluate) { - php_xpath_eval(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_EVALUATE, false); + dom_xpath_evaluate(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_EVALUATE, false); } PHP_METHOD(Dom_XPath, evaluate) { - php_xpath_eval(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_EVALUATE, true); + dom_xpath_evaluate(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_DOM_XPATH_EVALUATE, true); } /* }}} end dom_xpath_evaluate */ diff --git a/ext/dom/xpath_callbacks.c b/ext/dom/xpath_callbacks.c index e0f75fbf6a7c..9dcd4b37005f 100644 --- a/ext/dom/xpath_callbacks.c +++ b/ext/dom/xpath_callbacks.c @@ -48,6 +48,9 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_ctor(php_dom_xpath_callbacks *regist registry->php_ns = NULL; registry->namespaces = NULL; registry->node_list = NULL; + registry->read_only_documents = NULL; + registry->read_only_trees = NULL; + registry->read_only_depth = 0; } PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_node_list(php_dom_xpath_callbacks *registry) @@ -60,6 +63,81 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_node_list(php_dom_xpath_callba } } +static void php_dom_xpath_callbacks_make_read_only(php_dom_xpath_callbacks *registry, php_libxml_ref_obj *document) +{ + if (registry->read_only_depth == 0 || document == NULL) { + return; + } + + if (registry->read_only_documents == NULL) { + registry->read_only_documents = zend_new_array(0); + } + + if (zend_hash_index_add_ptr(registry->read_only_documents, (zend_ulong) (uintptr_t) document, document) != NULL) { + document->read_only_depth++; + document->refcount++; + } +} + +PHP_DOM_EXPORT void php_dom_xpath_callbacks_pin_tree(php_dom_xpath_callbacks *registry, xmlNodePtr node, dom_object *intern) +{ + if (registry->read_only_depth == 0 || node == NULL || node->type == XML_NAMESPACE_DECL) { + return; + } + + xmlNodePtr root = node; + while (root->parent != NULL) { + root = root->parent; + } + + if (root->type == XML_DOCUMENT_NODE || root->type == XML_HTML_DOCUMENT_NODE) { + return; + } + + zend_ulong key = (zend_ulong) (uintptr_t) root; + if (registry->read_only_trees != NULL && zend_hash_index_exists(registry->read_only_trees, key)) { + return; + } + + if (root->doc == NULL) { + intern = NULL; + } else if (php_dom_object_get_data(root) == NULL && (intern == NULL || intern->document == NULL || intern->document->ptr != root->doc)) { + return; + } + + zval root_zval; + php_dom_create_object(root, &root_zval, intern); + ((php_libxml_node_ptr *) root->_private)->read_only_depth++; + if (registry->read_only_trees == NULL) { + registry->read_only_trees = zend_new_array(0); + } + zend_hash_index_add_new(registry->read_only_trees, key, &root_zval); +} + +PHP_DOM_EXPORT void php_dom_xpath_callbacks_release_read_only(php_dom_xpath_callbacks *registry) +{ + if (registry->read_only_trees) { + HashTable *read_only_trees = registry->read_only_trees; + registry->read_only_trees = NULL; + zend_ulong key; + ZEND_HASH_FOREACH_NUM_KEY(read_only_trees, key) { + xmlNodePtr root = (xmlNodePtr) (uintptr_t) key; + ((php_libxml_node_ptr *) root->_private)->read_only_depth--; + } ZEND_HASH_FOREACH_END(); + zend_array_destroy(read_only_trees); + } + if (registry->read_only_documents) { + HashTable *read_only_documents = registry->read_only_documents; + registry->read_only_documents = NULL; + php_libxml_ref_obj *document; + ZEND_HASH_FOREACH_PTR(read_only_documents, document) { + document->read_only_depth--; + php_libxml_decrement_doc_ref_directly(document); + } ZEND_HASH_FOREACH_END(); + zend_array_destroy(read_only_documents); + } +} + PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_argument_stack(xmlXPathParserContextPtr ctxt, uint32_t num_args) { for (uint32_t i = 0; i < num_args; i++) { @@ -93,6 +171,7 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_dtor(php_dom_xpath_callbacks *regist FREE_HASHTABLE(namespaces); } php_dom_xpath_callbacks_clean_node_list(registry); + php_dom_xpath_callbacks_release_read_only(registry); } static void php_dom_xpath_callback_ns_get_gc(php_dom_xpath_callback_ns *ns, zend_get_gc_buffer *gc_buffer) @@ -323,8 +402,10 @@ PHP_DOM_EXPORT zend_result php_dom_xpath_callbacks_update_single_method_handler( return SUCCESS; } -static zval *php_dom_xpath_callback_fetch_args(xmlXPathParserContextPtr ctxt, uint32_t param_count, php_dom_xpath_nodeset_evaluation_mode evaluation_mode, dom_object *intern, php_dom_xpath_callbacks_proxy_factory proxy_factory) +static zval *php_dom_xpath_callback_fetch_args(php_dom_xpath_callbacks *xpath_callbacks, xmlXPathParserContextPtr ctxt, uint32_t param_count, php_dom_xpath_nodeset_evaluation_mode evaluation_mode, dom_object *intern, php_dom_xpath_callbacks_proxy_factory proxy_factory) { + php_dom_xpath_callbacks_pin_tree(xpath_callbacks, ctxt->context->node, intern); + if (param_count == 0) { return NULL; } @@ -354,6 +435,7 @@ static zval *php_dom_xpath_callback_fetch_args(xmlXPathParserContextPtr ctxt, ui if (obj->nodesetval && obj->nodesetval->nodeNr > 0) { array_init_size(param, obj->nodesetval->nodeNr); zend_hash_real_init_packed(Z_ARRVAL_P(param)); + xmlNodePtr previous = NULL; for (int j = 0; j < obj->nodesetval->nodeNr; j++) { xmlNodePtr node = obj->nodesetval->nodeTab[j]; zval child; @@ -369,6 +451,11 @@ static zval *php_dom_xpath_callback_fetch_args(xmlXPathParserContextPtr ctxt, ui php_dom_create_fake_namespace_decl(parent, original, &child, parent_intern); } else { proxy_factory(node, &child, intern, ctxt); + if (dom_object_get_node(Z_DOMOBJ_P(&child)) == node + && (previous == NULL || node->parent == NULL || (node->parent != previous && node->parent != previous->parent))) { + php_dom_xpath_callbacks_pin_tree(xpath_callbacks, node, Z_DOMOBJ_P(&child)); + } + previous = node; } zend_hash_next_index_insert_new(Z_ARRVAL_P(param), &child); } @@ -454,6 +541,10 @@ static zend_result php_dom_xpath_callback_dispatch(php_dom_xpath_callbacks *xpat zend_hash_next_index_insert_new(xpath_callbacks->node_list, &callback_retval); dom_object *obj = Z_DOMOBJ_P(&callback_retval); xmlNodePtr nodep = dom_object_get_node(obj); + if (nodep != NULL) { + php_dom_xpath_callbacks_make_read_only(xpath_callbacks, obj->document); + php_dom_xpath_callbacks_pin_tree(xpath_callbacks, nodep, obj); + } valuePush(ctxt, xmlXPathNewNodeSet(nodep)); } else if (Z_TYPE(callback_retval) == IS_FALSE || Z_TYPE(callback_retval) == IS_TRUE) { valuePush(ctxt, xmlXPathNewBoolean(Z_TYPE(callback_retval) == IS_TRUE)); @@ -481,7 +572,7 @@ PHP_DOM_EXPORT zend_result php_dom_xpath_callbacks_call_php_ns(php_dom_xpath_cal } uint32_t param_count = num_args - 1; - zval *params = php_dom_xpath_callback_fetch_args(ctxt, param_count, evaluation_mode, intern, proxy_factory); + zval *params = php_dom_xpath_callback_fetch_args(xpath_callbacks, ctxt, param_count, evaluation_mode, intern, proxy_factory); /* Last element of the stack is the function name */ xmlXPathObjectPtr obj = valuePop(ctxt); @@ -510,7 +601,7 @@ PHP_DOM_EXPORT zend_result php_dom_xpath_callbacks_call_php_ns(php_dom_xpath_cal PHP_DOM_EXPORT zend_result php_dom_xpath_callbacks_call_custom_ns(php_dom_xpath_callbacks *xpath_callbacks, xmlXPathParserContextPtr ctxt, int num_args, php_dom_xpath_nodeset_evaluation_mode evaluation_mode, dom_object *intern, php_dom_xpath_callbacks_proxy_factory proxy_factory) { uint32_t param_count = num_args; - zval *params = php_dom_xpath_callback_fetch_args(ctxt, param_count, evaluation_mode, intern, proxy_factory); + zval *params = php_dom_xpath_callback_fetch_args(xpath_callbacks, ctxt, param_count, evaluation_mode, intern, proxy_factory); const char *namespace = (const char *) ctxt->context->functionURI; /* Impossible because it wouldn't have been registered inside the context. */ diff --git a/ext/dom/xpath_callbacks.h b/ext/dom/xpath_callbacks.h index 4bf79b5df84f..a937b866cccc 100644 --- a/ext/dom/xpath_callbacks.h +++ b/ext/dom/xpath_callbacks.h @@ -41,6 +41,9 @@ typedef struct { php_dom_xpath_callback_ns *php_ns; HashTable *namespaces; HashTable *node_list; + HashTable *read_only_documents; + HashTable *read_only_trees; + uint32_t read_only_depth; } php_dom_xpath_callbacks; typedef enum { @@ -48,9 +51,16 @@ typedef enum { PHP_DOM_XPATH_CALLBACK_NAME_VALIDATE_NCNAME, } php_dom_xpath_callback_name_validation; +static zend_always_inline bool php_dom_xpath_callbacks_registered(const php_dom_xpath_callbacks *registry) +{ + return registry->php_ns != NULL || registry->namespaces != NULL; +} + PHP_DOM_EXPORT void php_dom_xpath_callbacks_ctor(php_dom_xpath_callbacks *registry); PHP_DOM_EXPORT void php_dom_xpath_callbacks_dtor(php_dom_xpath_callbacks *registry); PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_node_list(php_dom_xpath_callbacks *registry); +PHP_DOM_EXPORT void php_dom_xpath_callbacks_pin_tree(php_dom_xpath_callbacks *registry, xmlNodePtr node, dom_object *intern); +PHP_DOM_EXPORT void php_dom_xpath_callbacks_release_read_only(php_dom_xpath_callbacks *registry); PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_argument_stack(xmlXPathParserContextPtr ctxt, uint32_t num_args); PHP_DOM_EXPORT void php_dom_xpath_callbacks_get_gc(php_dom_xpath_callbacks *registry, zend_get_gc_buffer *gc_buffer); PHP_DOM_EXPORT HashTable *php_dom_xpath_callbacks_get_gc_for_whole_object(php_dom_xpath_callbacks *registry, zend_object *object, zval **table, int *n); diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c index d09ec299ff3c..10617d8c350c 100644 --- a/ext/libxml/libxml.c +++ b/ext/libxml/libxml.c @@ -1335,6 +1335,7 @@ PHP_LIBXML_API unsigned int php_libxml_increment_node_ptr(php_libxml_node_object ret_refcount = 1; object->node->node = node; object->node->refcount = 1; + object->node->read_only_depth = 0; object->node->_private = private_data; node->_private = object->node; } @@ -1387,6 +1388,7 @@ PHP_LIBXML_API unsigned int php_libxml_increment_doc_ref(php_libxml_node_object object->document->class_type = PHP_LIBXML_CLASS_UNSET; object->document->handlers = &php_libxml_default_document_handlers; object->document->quirks_mode = PHP_LIBXML_NO_QUIRKS; + object->document->read_only_depth = 0; } return ret_refcount; @@ -1427,6 +1429,20 @@ PHP_LIBXML_API unsigned int php_libxml_decrement_doc_ref(php_libxml_node_object return ret_refcount; } +PHP_LIBXML_API bool php_libxml_is_tree_read_only(const php_libxml_ref_obj *document, const xmlNode *node) +{ + while (node->parent != NULL) { + node = node->parent; + } + + if (node->type == XML_DOCUMENT_NODE || node->type == XML_HTML_DOCUMENT_NODE) { + return document != NULL && document->read_only_depth > 0; + } + + const php_libxml_node_ptr *ptr = node->_private; + return ptr != NULL && ptr->read_only_depth > 0; +} + PHP_LIBXML_API void php_libxml_node_free_resource(xmlNodePtr node) { if (!node) { diff --git a/ext/libxml/php_libxml.h b/ext/libxml/php_libxml.h index 5896f70803f8..89e35b710f9a 100644 --- a/ext/libxml/php_libxml.h +++ b/ext/libxml/php_libxml.h @@ -114,11 +114,13 @@ typedef struct _php_libxml_ref_obj { unsigned int refcount; php_libxml_class_type class_type : 8; php_libxml_quirks_mode quirks_mode : 8; + unsigned int read_only_depth; } php_libxml_ref_obj; typedef struct _php_libxml_node_ptr { xmlNodePtr node; unsigned int refcount; + unsigned int read_only_depth; void *_private; } php_libxml_node_ptr; @@ -166,6 +168,16 @@ static zend_always_inline void php_libxml_invalidate_node_list_cache(php_libxml_ } } +PHP_LIBXML_API bool php_libxml_is_tree_read_only(const php_libxml_ref_obj *document, const xmlNode *node); + +static zend_always_inline bool php_libxml_is_node_read_only(const php_libxml_ref_obj *document, const xmlNode *node) +{ + if (node == NULL || (document != NULL && EXPECTED(document->read_only_depth == 0))) { + return false; + } + return php_libxml_is_tree_read_only(document, node); +} + static zend_always_inline void php_libxml_invalidate_node_list_cache_from_doc(xmlDocPtr docp) { if (docp && docp->_private) { /* docp is NULL for detached nodes */ diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 0e02a843e95a..268a0e02a465 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -373,6 +373,15 @@ static void change_node_zval(xmlNodePtr node, zend_string *value) } /* }}} */ +static bool sxe_throw_if_document_read_only(const php_sxe_object *sxe) +{ + if (php_libxml_is_node_read_only(sxe->document, sxe->node ? sxe->node->node : NULL)) { + zend_throw_error(NULL, "Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress"); + return true; + } + return false; +} + /* {{{ sxe_property_write() */ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value, sxe_access_mode access_mode, xmlNodePtr *pnewnode) { @@ -393,6 +402,10 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value, sxe = php_sxe_fetch_object(object); + if (sxe_throw_if_document_read_only(sxe)) { + return &EG(error_zval); + } + if (!member) { if (sxe->iter.type == SXE_ITER_ATTRLIST) { /* This happens when the user did: $sxe[] = $value @@ -796,6 +809,12 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, sxe_access_mo zval tmp_zv; int test = 0; + sxe = php_sxe_fetch_object(object); + + if (sxe_throw_if_document_read_only(sxe)) { + return; + } + if (Z_TYPE_P(member) != IS_STRING && Z_TYPE_P(member) != IS_LONG) { zend_string *str = zval_try_get_string_func(member); if (UNEXPECTED(!str)) { @@ -805,8 +824,6 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, sxe_access_mo member = &tmp_zv; } - sxe = php_sxe_fetch_object(object); - GET_NODE(sxe, node); if (Z_TYPE_P(member) == IS_LONG) { @@ -1732,6 +1749,10 @@ PHP_METHOD(SimpleXMLElement, addChild) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); + if (sxe_throw_if_document_read_only(sxe)) { + RETURN_THROWS(); + } + if (sxe->iter.type == SXE_ITER_ATTRLIST) { php_error_docref(NULL, E_WARNING, "Cannot add element to attributes"); return; @@ -1807,6 +1828,10 @@ PHP_METHOD(SimpleXMLElement, addAttribute) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); + if (sxe_throw_if_document_read_only(sxe)) { + RETURN_THROWS(); + } + node = php_sxe_get_first_node_non_destructive(sxe, node); if (node && node->type != XML_ELEMENT_NODE) { From ae17964d8887ad7514262931820bc2fcda97bd1b Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 9 Nov 2026 19:29:35 -0500 Subject: [PATCH 3/3] ext/xsl: Fix use-after-free when an XSLT callback modifies the source document A php:function() callback, or an entity loader or error handler, runs while libxslt walks the source tree, so modifying the tree or dropping the last reference to a detached subtree freed nodes the transformation still referenced. Make the source tree read-only for the duration of every transformation, and arm the XPath callback registry so that, as for DOMXPath, the trees of returned nodes and the detached subtrees of the current node and of callback arguments are read-only and kept alive until the transformation ends. --- NEWS | 3 + UPGRADING | 8 + ext/xsl/tests/bug69168.phpt | 14 +- ext/xsl/tests/cloneDocument.phpt | 7 +- ...ocessor_entity_loader_modifies_source.phpt | 42 +++++ .../xsltprocessor_error_handler_bailout.phpt | 39 +++++ .../xsltprocessor_php_function_bailout.phpt | 55 +++++++ ...rocessor_php_function_modifies_source.phpt | 144 ++++++++++++++++++ ext/xsl/xsltprocessor.c | 6 + 9 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 ext/xsl/tests/xsltprocessor_entity_loader_modifies_source.phpt create mode 100644 ext/xsl/tests/xsltprocessor_error_handler_bailout.phpt create mode 100644 ext/xsl/tests/xsltprocessor_php_function_bailout.phpt create mode 100644 ext/xsl/tests/xsltprocessor_php_function_modifies_source.phpt diff --git a/NEWS b/NEWS index 734f986001af..841a7f49349f 100644 --- a/NEWS +++ b/NEWS @@ -212,6 +212,9 @@ PHP NEWS - XSL: . Fixed a use-after-free when a transformation was started on a processor that is already transforming. (Ilia Alshanetsky) + . Fixed use-after-free when a php:function() callback or an entity loader + modifies the document being transformed; such modifications now throw a + DOMException or Error. (Ilia Alshanetsky) - Zlib: . Fixed inflate_init() dropping the preset dictionary for raw streams with diff --git a/UPGRADING b/UPGRADING index ee2184d46a27..42e4f845e463 100644 --- a/UPGRADING +++ b/UPGRADING @@ -81,6 +81,14 @@ PHP 8.7 UPGRADE NOTES same processor is already transforming, for example from a php:function() callback or from a suspended Fiber. Use a separate XSLTProcessor instance instead. + . While XSLTProcessor transforms a document, the tree of that document, the + tree of every node a php:function() callback returned, and the detached + subtree of the current node or of a node passed to a callback are + read-only. Modifying such a node, for example from a callback or an entity + loader, throws a DOMException with code DOM_NO_MODIFICATION_ALLOWED_ERR (a + warning when strictErrorChecking is disabled), and modifying it through + SimpleXML throws an Error. Set XSLTProcessor::$cloneDocument to true to + transform a copy and keep the original document modifiable. ======================================== 2. New Features diff --git a/ext/xsl/tests/bug69168.phpt b/ext/xsl/tests/bug69168.phpt index 41fc3aafd820..4f1f44c11561 100644 --- a/ext/xsl/tests/bug69168.phpt +++ b/ext/xsl/tests/bug69168.phpt @@ -19,7 +19,11 @@ $xsl = <<nodeValue .= 'a'; + try { + $input[0]->nodeValue .= 'a'; + } catch (DOMException $e) { + echo $e->getMessage(), "\n"; + } return $input[0]->getNodePath() . ' = ' . $input[0]->nodeValue; } @@ -32,12 +36,14 @@ $xmlDoc = new DOMDocument(); $xmlDoc->loadXML($xml); echo @$proc->transformToXML($xmlDoc); -// Tests modification of the nodes +// Tests that the callback did not modify the nodes var_dump($xmlDoc->firstChild->firstChild->firstChild->getNodePath()); var_dump($xmlDoc->firstChild->firstChild->firstChild->nodeValue); ?> --EXPECT-- +Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress -/allusers/user[1]/uid = boba
/allusers/user[2]/uid = joea
+/allusers/user[1]/uid = bob
/allusers/user[2]/uid = joe
string(21) "/allusers/user[1]/uid" -string(4) "boba" +string(3) "bob" diff --git a/ext/xsl/tests/cloneDocument.phpt b/ext/xsl/tests/cloneDocument.phpt index 0af1da4c6e35..153346ee6f01 100644 --- a/ext/xsl/tests/cloneDocument.phpt +++ b/ext/xsl/tests/cloneDocument.phpt @@ -11,7 +11,11 @@ $xml->loadXML('hello'); function test() { global $xml; - $xml->documentElement->firstChild->textContent = "bye"; + try { + $xml->documentElement->firstChild->textContent = "bye"; + } catch (DOMException $e) { + echo $e->getMessage(), "\n"; + } } $xsl = new DOMDocument; @@ -41,5 +45,6 @@ echo $xslt->transformToXml($xml); --EXPECT-- hello +Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress bye diff --git a/ext/xsl/tests/xsltprocessor_entity_loader_modifies_source.phpt b/ext/xsl/tests/xsltprocessor_entity_loader_modifies_source.phpt new file mode 100644 index 000000000000..483cc9d92592 --- /dev/null +++ b/ext/xsl/tests/xsltprocessor_entity_loader_modifies_source.phpt @@ -0,0 +1,42 @@ +--TEST-- +XSLTProcessor: an entity loader cannot modify the document that is being transformed +--EXTENSIONS-- +xsl +--FILE-- +loadXML('123'); + +libxml_set_external_entity_loader(function ($public, $system, $context) use ($doc) { + try { + $doc->documentElement->textContent = ''; + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + $stream = fopen('php://memory', 'w+'); + fwrite($stream, 'loaded'); + rewind($stream); + return $stream; +}); + +$xsl = new DOMDocument(); +$xsl->loadXML(<< + + +[|] + + +XSL); +$proc = new XSLTProcessor(); +$proc->importStylesheet($xsl); +var_dump($proc->transformToXml($doc)); +$doc->documentElement->textContent = ''; +echo $doc->saveXML($doc->documentElement), "\n"; +?> +--EXPECT-- +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +string(30) "[loaded|1][loaded|2][loaded|3]" + diff --git a/ext/xsl/tests/xsltprocessor_error_handler_bailout.phpt b/ext/xsl/tests/xsltprocessor_error_handler_bailout.phpt new file mode 100644 index 000000000000..93b482ba98a4 --- /dev/null +++ b/ext/xsl/tests/xsltprocessor_error_handler_bailout.phpt @@ -0,0 +1,39 @@ +--TEST-- +XSLTProcessor: the source becomes writable again after a fatal error in user code during a transformation +--EXTENSIONS-- +xsl +zend_test +--SKIPIF-- + +--FILE-- +loadXML(''); + +register_shutdown_function(function () use ($doc) { + $doc->documentElement->firstChild->remove(); + echo $doc->saveXML($doc->documentElement), "\n"; +}); + +set_error_handler(function () use ($doc) { + try { + $doc->documentElement->firstChild->remove(); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + zend_trigger_bailout(); +}); + +$xsl = new DOMDocument(); +$xsl->loadXML('m'); +$proc = new XSLTProcessor(); +$proc->importStylesheet($xsl); +$proc->transformToXml($doc); +?> +--EXPECTF-- +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress + +Fatal error: Bailout in %s on line %d + diff --git a/ext/xsl/tests/xsltprocessor_php_function_bailout.phpt b/ext/xsl/tests/xsltprocessor_php_function_bailout.phpt new file mode 100644 index 000000000000..a8160df404d1 --- /dev/null +++ b/ext/xsl/tests/xsltprocessor_php_function_bailout.phpt @@ -0,0 +1,55 @@ +--TEST-- +XSLTProcessor: documents become writable again after a fatal error in a php:function() callback +--EXTENSIONS-- +xsl +zend_test +--SKIPIF-- + +--FILE-- +loadXML('1'); +$other = new DOMDocument(); +$other->loadXML(''); + +function other_root() { + return $GLOBALS['other']->documentElement; +} + +function bail() { + try { + $GLOBALS['other']->documentElement->firstChild->remove(); + } catch (DOMException $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; + } + zend_trigger_bailout(); +} + +function stylesheet(string $select): DOMDocument { + $xsl = new DOMDocument(); + $xsl->loadXML(''); + return $xsl; +} + +$proc = new XSLTProcessor(); +$proc->registerPHPFunctions(); +$proc->importStylesheet(stylesheet("concat(count(php:function('other_root')), php:function('bail'))")); + +register_shutdown_function(function () use ($proc, $doc, $other) { + $doc->documentElement->firstChild->remove(); + $other->documentElement->firstChild->remove(); + echo $doc->saveXML($doc->documentElement), $other->saveXML($other->documentElement), "\n"; + $proc->importStylesheet(stylesheet("count(php:function('other_root'))")); + echo $proc->transformToXml($doc), "\n"; +}); + +$proc->transformToXml($doc); +?> +--EXPECTF-- +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress + +Fatal error: Bailout in %s on line %d + +1 diff --git a/ext/xsl/tests/xsltprocessor_php_function_modifies_source.phpt b/ext/xsl/tests/xsltprocessor_php_function_modifies_source.phpt new file mode 100644 index 000000000000..1ffa183a5f31 --- /dev/null +++ b/ext/xsl/tests/xsltprocessor_php_function_modifies_source.phpt @@ -0,0 +1,144 @@ +--TEST-- +XSLTProcessor: a php:function() callback cannot modify the document that is being transformed +--EXTENSIONS-- +xsl +--FILE-- +getMessage()] = true; + } + return ''; +} + +function show($nodes) { + return $nodes[0]->nodeName . ': ' . $nodes[0]->textContent . "\n"; +} + +function transform(string $label, DOMDocument $doc, string $select, Closure $mutation) { + $GLOBALS['mutation'] = $mutation; + $GLOBALS['errors'] = []; + $xsl = new DOMDocument(); + $xsl->loadXML(<< + + + + + + XSL); + $proc = new XSLTProcessor(); + $proc->importStylesheet($xsl); + $proc->registerPHPFunctions(); + echo "$label:\n", $proc->transformToXml($doc); + echo implode("\n", array_keys($GLOBALS['errors'])), "\n"; +} + +$doc = new DOMDocument(); +$doc->loadXML('123'); +$root = $doc->documentElement; +transform('removeChild', $doc, '/root/a', function () use ($root) { + foreach (iterator_to_array($root->childNodes) as $child) { + $root->removeChild($child); + } +}); +transform('textContent', $doc, '/root/a', function () use ($root) { + $root->textContent = 'x'; +}); +transform('removeAttribute', $doc, '/root/@*', function () use ($root) { + $root->removeAttribute('a'); +}); +echo $doc->saveXML($root), "\n"; + +$text = new DOMDocument(); +$text->loadXML(''); +foreach (['1', '2', '3'] as $data) { + $text->documentElement->appendChild($text->createTextNode($data)); +} +transform('normalize', $text, '/root/text()', function () use ($text) { + $text->documentElement->normalize(); +}); +echo $text->saveXML($text->documentElement), "\n"; + +$other = new DOMDocument(); +$other->loadXML('123'); +function other_root() { + return $GLOBALS['other']->documentElement; +} +transform('foreign document', $doc, "php:function('other_root')/a", function () use ($other) { + $other->documentElement->firstChild->remove(); +}); +echo $other->saveXML($other->documentElement), "\n"; + +$root->firstChild->remove(); +$other->documentElement->firstChild->remove(); +echo $doc->saveXML($root), "\n", $other->saveXML($other->documentElement), "\n"; + +function wrap($nodes) { + $doc = $nodes[0]->ownerDocument; + $span = $doc->createElement('span'); + $span->setAttribute('class', 'w'); + $span->appendChild($doc->createTextNode(strtoupper($nodes[0]->textContent))); + return $span; +} +$docless = new DOMElement('e', 'abc'); +function docless() { + return $GLOBALS['docless']; +} +function modify_docless() { + try { + $GLOBALS['docless']->textContent = 'xyz'; + } catch (DOMException $e) { + $GLOBALS['errors'][$e::class . ": " . $e->getMessage()] = true; + } + return ''; +} +$GLOBALS['errors'] = []; +$xsl = new DOMDocument(); +$xsl->loadXML(<< + + + + + +XSL); +$proc = new XSLTProcessor(); +$proc->importStylesheet($xsl); +$proc->registerPHPFunctions(); +echo $proc->transformToXml($doc), implode("\n", array_keys($GLOBALS['errors'])), "\n"; +?> +--EXPECT-- +removeChild: +a: 1 +a: 2 +a: 3 +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +textContent: +a: 1 +a: 2 +a: 3 +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +removeAttribute: +a: 1 +b: 2 +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +123 +normalize: +#text: 1 +#text: 2 +#text: 3 +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +123 +foreign document: +a: 1 +a: 2 +a: 3 +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress +123 +23 +23 +23abc +DOMException: Cannot modify a node while an XPath evaluation or XSLT transformation that uses it is in progress diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index f66ae07a57b3..1e76f383e554 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -417,11 +417,17 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl if (secPrefsError == 1) { php_error_docref(NULL, E_WARNING, "Can't set libxslt security properties, not doing transformation for security reasons"); } else { + php_libxml_ref_obj *document = intern->doc->document; + document->read_only_depth++; + intern->xpath_callbacks.read_only_depth++; zend_try { newdocp = xsltApplyStylesheetUser(style, doc, NULL, NULL, f, ctxt); } zend_catch { bailout = true; } zend_end_try(); + intern->xpath_callbacks.read_only_depth--; + document->read_only_depth--; + php_dom_xpath_callbacks_release_read_only(&intern->xpath_callbacks); } out: