Fix CVE-2016-5131: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-5131 Patches copied from upstream source repository (the test suite fails without the 2nd patch): https://git.gnome.org/browse/libxml2/commit/?id=9ab01a277d71f54d3143c2cf333c5c2e9aaedd9e https://git.gnome.org/browse/libxml2/commit/?id=a005199330b86dada19d162cae15ef9bdcb6baa8 From 9ab01a277d71f54d3143c2cf333c5c2e9aaedd9e Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Tue, 28 Jun 2016 14:22:23 +0200 Subject: [PATCH] Fix XPointer paths beginning with range-to The old code would invoke the broken xmlXPtrRangeToFunction. range-to isn't really a function but a special kind of location step. Remove this function and always handle range-to in the XPath code. The old xmlXPtrRangeToFunction could also be abused to trigger a use-after-free error with the potential for remote code execution. Found with afl-fuzz. Fixes CVE-2016-5131. --- result/XPath/xptr/vidbase | 13 ++++++++ test/XPath/xptr/vidbase | 1 + xpath.c | 7 ++++- xpointer.c | 76 ++++------------------------------------------- 4 files changed, 26 insertions(+), 71 deletions(-) diff --git a/result/XPath/xptr/vidbase b/result/XPath/xptr/vidbase index 8b9e92d6..f19193e7 100644 --- a/result/XPath/xptr/vidbase +++ b/result/XPath/xptr/vidbase @@ -17,3 +17,16 @@ Object is a Location Set: To node ELEMENT p + +======================== +Expression: xpointer(range-to(id('chapter2'))) +Object is a Location Set: +1 : Object is a range : + From node + / + To node + ELEMENT chapter + ATTRIBUTE id + TEXT + content=chapter2 + diff --git a/test/XPath/xptr/vidbase b/test/XPath/xptr/vidbase index b1463830..884b1065 100644 --- a/test/XPath/xptr/vidbase +++ b/test/XPath/xptr/vidbase @@ -1,2 +1,3 @@ xpointer(id('chapter1')/p) xpointer(id('chapter1')/p[1]/range-to(following-sibling::p[2])) +xpointer(range-to(id('chapter2'))) diff --git a/xpath.c b/xpath.c index d992841e..5a01b1b3 100644 --- a/xpath.c +++ b/xpath.c @@ -10691,13 +10691,18 @@ xmlXPathCompPathExpr(xmlXPathParserContextPtr ctxt) { lc = 1; break; } else if ((NXT(len) == '(')) { - /* Note Type or Function */ + /* Node Type or Function */ if (xmlXPathIsNodeType(name)) { #ifdef DEBUG_STEP xmlGenericError(xmlGenericErrorContext, "PathExpr: Type search\n"); #endif lc = 1; +#ifdef LIBXML_XPTR_ENABLED + } else if (ctxt->xptr && + xmlStrEqual(name, BAD_CAST "range-to")) { + lc = 1; +#endif } else { #ifdef DEBUG_STEP xmlGenericError(xmlGenericErrorContext, diff --git a/xpointer.c b/xpointer.c index 676c5105..d74174a3 100644 --- a/xpointer.c +++ b/xpointer.c @@ -1332,8 +1332,6 @@ xmlXPtrNewContext(xmlDocPtr doc, xmlNodePtr here, xmlNodePtr origin) { ret->here = here; ret->origin = origin; - xmlXPathRegisterFunc(ret, (xmlChar *)"range-to", - xmlXPtrRangeToFunction); xmlXPathRegisterFunc(ret, (xmlChar *)"range", xmlXPtrRangeFunction); xmlXPathRegisterFunc(ret, (xmlChar *)"range-inside", @@ -2243,76 +2241,14 @@ xmlXPtrRangeInsideFunction(xmlXPathParserContextPtr ctxt, int nargs) { * @nargs: the number of args * * Implement the range-to() XPointer function + * + * Obsolete. range-to is not a real function but a special type of location + * step which is handled in xpath.c. */ void -xmlXPtrRangeToFunction(xmlXPathParserContextPtr ctxt, int nargs) { - xmlXPathObjectPtr range; - const xmlChar *cur; - xmlXPathObjectPtr res, obj; - xmlXPathObjectPtr tmp; - xmlLocationSetPtr newset = NULL; - xmlNodeSetPtr oldset; - int i; - - if (ctxt == NULL) return; - CHECK_ARITY(1); - /* - * Save the expression pointer since we will have to evaluate - * it multiple times. Initialize the new set. - */ - CHECK_TYPE(XPATH_NODESET); - obj = valuePop(ctxt); - oldset = obj->nodesetval; - ctxt->context->node = NULL; - - cur = ctxt->cur; - newset = xmlXPtrLocationSetCreate(NULL); - - for (i = 0; i < oldset->nodeNr; i++) { - ctxt->cur = cur; - - /* - * Run the evaluation with a node list made of a single item - * in the nodeset. - */ - ctxt->context->node = oldset->nodeTab[i]; - tmp = xmlXPathNewNodeSet(ctxt->context->node); - valuePush(ctxt, tmp); - - xmlXPathEvalExpr(ctxt); - CHECK_ERROR; - - /* - * The result of the evaluation need to be tested to - * decided whether the filter succeeded or not - */ - res = valuePop(ctxt); - range = xmlXPtrNewRangeNodeObject(oldset->nodeTab[i], res); - if (range != NULL) { - xmlXPtrLocationSetAdd(newset, range); - } - - /* - * Cleanup - */ - if (res != NULL) - xmlXPathFreeObject(res); - if (ctxt->value == tmp) { - res = valuePop(ctxt); - xmlXPathFreeObject(res); - } - - ctxt->context->node = NULL; - } - - /* - * The result is used as the new evaluation set. - */ - xmlXPathFreeObject(obj); - ctxt->context->node = NULL; - ctxt->context->contextSize = -1; - ctxt->context->proximityPosition = -1; - valuePush(ctxt, xmlXPtrWrapLocationSet(newset)); +xmlXPtrRangeToFunction(xmlXPathParserContextPtr ctxt, + int nargs ATTRIBUTE_UNUSED) { + XP_ERROR(XPATH_EXPR_ERROR); } /** -- 2.11.0 From a005199330b86dada19d162cae15ef9bdcb6baa8 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Tue, 28 Jun 2016 14:19:58 +0200 Subject: [PATCH] Fix comparison with root node in xmlXPathCmpNodes This change has already been made in xmlXPathCmpNodesExt but not in xmlXPathCmpNodes. --- xpath.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xpath.c b/xpath.c index 751665b8..d992841e 100644 --- a/xpath.c +++ b/xpath.c @@ -3342,13 +3342,13 @@ xmlXPathCmpNodes(xmlNodePtr node1, xmlNodePtr node2) { * compute depth to root */ for (depth2 = 0, cur = node2;cur->parent != NULL;cur = cur->parent) { - if (cur == node1) + if (cur->parent == node1) return(1); depth2++; } root = cur; for (depth1 = 0, cur = node1;cur->parent != NULL;cur = cur->parent) { - if (cur == node2) + if (cur->parent == node2) return(-1); depth1++; } -- 2.11.0