From 08b62c25871b38d5d573515ca8a065b4b8f64f6b Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Wed, 20 Feb 2019 13:24:37 +0100 Subject: [PATCH 31/33] Always set context node before calling XPath iterators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The xmlXPathNext* iterators rely on the XPath context node being set to the start node of the iteration. Some parts of the code base like the xsl:key functions also leave the context node in an unspecified state. Make sure that the context node is reset before invoking the XPath iterators. Also backup and restore the context node in xsltNumberFormatGetMultipleLevel for good measure. This bug could also lead to type confusion and invalid reads in connection with namespace nodes. Fixes #13. Also see the Chromium bug report: https://bugs.chromium.org/p/chromium/issues/detail?id=930663 Thanks to Nicolas Grégoire for the report. --- libxslt/numbers.c | 31 ++++++++++++++++++++----------- tests/docs/bug-218.xml | 1 + tests/general/bug-218.out | 2 ++ tests/general/bug-218.xsl | 8 ++++++++ 4 files changed, 31 insertions(+), 11 deletions(-) create mode 100644 tests/docs/bug-218.xml create mode 100644 tests/general/bug-218.out create mode 100644 tests/general/bug-218.xsl diff --git a/libxslt/numbers.c b/libxslt/numbers.c index 0d34740..89e1f66 100644 --- a/libxslt/numbers.c +++ b/libxslt/numbers.c @@ -646,42 +646,51 @@ xsltNumberFormatGetMultipleLevel(xsltTransformContextPtr context, { int amount = 0; int cnt; + xmlNodePtr oldCtxtNode; xmlNodePtr ancestor; xmlNodePtr preceding; xmlXPathParserContextPtr parser; - context->xpathCtxt->node = node; + oldCtxtNode = context->xpathCtxt->node; parser = xmlXPathNewParserContext(NULL, context->xpathCtxt); if (parser) { /* ancestor-or-self::*[count] */ - for (ancestor = node; - (ancestor != NULL) && (ancestor->type != XML_DOCUMENT_NODE); - ancestor = xmlXPathNextAncestor(parser, ancestor)) { - + ancestor = node; + while ((ancestor != NULL) && (ancestor->type != XML_DOCUMENT_NODE)) { if ((fromPat != NULL) && xsltTestCompMatchList(context, ancestor, fromPat)) break; /* for */ + /* + * The xmlXPathNext* iterators require that the context node is + * set to the start node. Calls to xsltTestCompMatch* may also + * leave the context node in an undefined state, so make sure + * that the context node is reset before each iterator invocation. + */ + if (xsltTestCompMatchCount(context, ancestor, countPat, node)) { /* count(preceding-sibling::*) */ cnt = 1; - for (preceding = - xmlXPathNextPrecedingSibling(parser, ancestor); - preceding != NULL; - preceding = - xmlXPathNextPrecedingSibling(parser, preceding)) { - + context->xpathCtxt->node = ancestor; + preceding = xmlXPathNextPrecedingSibling(parser, ancestor); + while (preceding != NULL) { if (xsltTestCompMatchCount(context, preceding, countPat, node)) cnt++; + context->xpathCtxt->node = ancestor; + preceding = + xmlXPathNextPrecedingSibling(parser, preceding); } array[amount++] = (double)cnt; if (amount >= max) break; /* for */ } + context->xpathCtxt->node = node; + ancestor = xmlXPathNextAncestor(parser, ancestor); } xmlXPathFreeParserContext(parser); } + context->xpathCtxt->node = oldCtxtNode; return amount; } diff --git a/tests/docs/bug-218.xml b/tests/docs/bug-218.xml new file mode 100644 index 0000000..3806547 --- /dev/null +++ b/tests/docs/bug-218.xml @@ -0,0 +1 @@ + diff --git a/tests/general/bug-218.out b/tests/general/bug-218.out new file mode 100644 index 0000000..832a29e --- /dev/null +++ b/tests/general/bug-218.out @@ -0,0 +1,2 @@ + +1 diff --git a/tests/general/bug-218.xsl b/tests/general/bug-218.xsl new file mode 100644 index 0000000..fdbb7b1 --- /dev/null +++ b/tests/general/bug-218.xsl @@ -0,0 +1,8 @@ + + + + + + + + -- 1.8.3.1