From 1925855c0fd14e245fe185d1cfb44bf3c29b4e14 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 14 Jan 2023 16:21:35 +0100
Subject: [PATCH] Fix bug 69168: DomNode::getNodePath() returns invalid path

Upon freeing libxslt's context, every document which is not the *main*
document will be freed by libxslt. If a node of a document which is not
the main document gets returned to userland, we'd free the node twice:
 - first by the cleanup of the xslt context
 - and then by our own refcounting mechanism.
This was reported in bug 49634, and was fixed by always copying the
node (and later re-fixed in bug 70078).
The original fix is not entirely correct unfortunately because of the
following two main reasons:
 - modifications to the node will only modify the copy, and not the original
 - accesses to the parent, path, ... will not work

This patch fixes it properly by only copying the node if it origins from
a document other than the main document.

Co-authored-by: juha.ikavalko@agentit.fi

Closes GH-10318.
---
 NEWS                        |  3 +++
 ext/xsl/tests/bug69168.phpt | 43 +++++++++++++++++++++++++++++++++++++
 ext/xsl/xsltprocessor.c     | 14 +++++++++++-
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 ext/xsl/tests/bug69168.phpt

diff --git a/NEWS b/NEWS
index 76dd04a4ac9..ee4dcfc207d 100644
--- a/NEWS
+++ b/NEWS
@@ -98,4 +98,7 @@ PHP                                                                        NEWS
   . Fixed bug #51056: blocking fread() will block even if data is available.
     (Jakub Zelenka)
 
+- XSLTProcessor:
+  . Fixed bug #69168 (DomNode::getNodePath() returns invalid path). (nielsdos)
+
 <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>
diff --git a/ext/xsl/tests/bug69168.phpt b/ext/xsl/tests/bug69168.phpt
new file mode 100644
index 00000000000..41fc3aafd82
--- /dev/null
+++ b/ext/xsl/tests/bug69168.phpt
@@ -0,0 +1,43 @@
+--TEST--
+bug #69168 (DomNode::getNodePath() returns invalid path)
+--EXTENSIONS--
+xsl
+--FILE--
+<?php
+$xml = <<<EOB
+<allusers><user><uid>bob</uid></user><user><uid>joe</uid></user></allusers>
+EOB;
+$xsl = <<<EOB
+<?xml version="1.0" encoding="UTF-8"?>
+<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:php="http://php.net/xsl">
+ <xsl:template match="allusers">
+    <xsl:for-each select="user">
+      <xsl:value-of select="php:function('getPath',uid)"/><br />
+    </xsl:for-each>
+ </xsl:template>
+</xsl:stylesheet>
+EOB;
+
+function getPath($input){
+  $input[0]->nodeValue .= 'a';
+  return $input[0]->getNodePath() . ' = ' . $input[0]->nodeValue;
+}
+
+$proc = new XSLTProcessor();
+$proc->registerPHPFunctions();
+$xslDoc = new DOMDocument();
+$xslDoc->loadXML($xsl);
+@$proc->importStyleSheet($xslDoc);
+$xmlDoc = new DOMDocument();
+$xmlDoc->loadXML($xml);
+echo @$proc->transformToXML($xmlDoc);
+
+// Tests modification of the nodes
+var_dump($xmlDoc->firstChild->firstChild->firstChild->getNodePath());
+var_dump($xmlDoc->firstChild->firstChild->firstChild->nodeValue);
+?>
+--EXPECT--
+<?xml version="1.0"?>
+/allusers/user[1]/uid = boba<br/>/allusers/user[2]/uid = joea<br/>
+string(21) "/allusers/user[1]/uid"
+string(4) "boba"
diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c
index c62c0a13cea..8dbe6b27a11 100644
--- a/ext/xsl/xsltprocessor.c
+++ b/ext/xsl/xsltprocessor.c
@@ -194,7 +194,19 @@ static void xsl_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, int t
 								node->parent = nsparent;
 								node->ns = curns;
 							} else {
-								node = xmlDocCopyNode(node, domintern->document->ptr, 1);
+								/**
+								 * Upon freeing libxslt's context, every document which is not the *main* document will be freed by libxslt.
+								 * If a node of a document which is *not the main* document gets returned to userland, we'd free the node twice:
+								 * first by the cleanup of the xslt context, and then by our own refcounting mechanism.
+								 * To prevent this, we'll take a copy if the node is not from the main document.
+								 * It is important that we do not copy the node unconditionally, because that means that:
+								 *  - modifications to the node will only modify the copy, and not the original
+								 *  - accesses to the parent, path, ... will not work
+								 */
+								xsltTransformContextPtr transform_ctxt = (xsltTransformContextPtr) ctxt->context->extra;
+								if (node->doc != transform_ctxt->document->doc) {
+									node = xmlDocCopyNode(node, domintern->document->ptr, 1);
+								}
 							}
 
 							php_dom_create_object(node, &child, domintern);