Skip to content

Commit

Permalink
DO NOT MERGE: Fix XPointer paths beginning with range-to
Browse files Browse the repository at this point in the history
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.

Bug: 36554209
Change-Id: I2bd369290a884c432d16796884d48db6285f8502
(cherry picked from commit e875e1cd1fc92fd2daa57826024125cbd0b195c7)
  • Loading branch information
Brian C. Young authored and andi34 committed Jun 16, 2017
1 parent a2bb4a2 commit f8ebef2
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 71 deletions.
7 changes: 6 additions & 1 deletion xpath.c
Expand Up @@ -10596,13 +10596,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,
Expand Down
76 changes: 6 additions & 70 deletions xpointer.c
Expand Up @@ -1339,8 +1339,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",
Expand Down Expand Up @@ -2226,76 +2224,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);
}

/**
Expand Down

0 comments on commit f8ebef2

Please sign in to comment.