From c1d1f7121194036608bf555f08d3062a36fd344b Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Tue, 28 Jun 2016 18:34:52 +0200 Subject: [PATCH] Disallow namespace nodes in XPointer ranges Namespace nodes must be copied to avoid use-after-free errors. But they don't necessarily have a physical representation in a document, so simply disallow them in XPointer ranges. Found with afl-fuzz. Fixes CVE-2016-4658. --- xpointer.c | 149 ++++++++++++++++++++--------------------------------- 1 file changed, 56 insertions(+), 93 deletions(-) diff --git a/xpointer.c b/xpointer.c index a7b03fbd..694d120e 100644 --- a/xpointer.c +++ b/xpointer.c @@ -319,6 +319,45 @@ xmlXPtrRangesEqual(xmlXPathObjectPtr range1, xmlXPathObjectPtr range2) { return(1); } +/** + * xmlXPtrNewRangeInternal: + * @start: the starting node + * @startindex: the start index + * @end: the ending point + * @endindex: the ending index + * + * Internal function to create a new xmlXPathObjectPtr of type range + * + * Returns the newly created object. + */ +static xmlXPathObjectPtr +xmlXPtrNewRangeInternal(xmlNodePtr start, int startindex, + xmlNodePtr end, int endindex) { + xmlXPathObjectPtr ret; + + /* + * Namespace nodes must be copied (see xmlXPathNodeSetDupNs). + * Disallow them for now. + */ + if ((start != NULL) && (start->type == XML_NAMESPACE_DECL)) + return(NULL); + if ((end != NULL) && (end->type == XML_NAMESPACE_DECL)) + return(NULL); + + ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); + if (ret == NULL) { + xmlXPtrErrMemory("allocating range"); + return(NULL); + } + memset(ret, 0, sizeof(xmlXPathObject)); + ret->type = XPATH_RANGE; + ret->user = start; + ret->index = startindex; + ret->user2 = end; + ret->index2 = endindex; + return(ret); +} + /** * xmlXPtrNewRange: * @start: the starting node @@ -344,17 +383,7 @@ xmlXPtrNewRange(xmlNodePtr start, int startindex, if (endindex < 0) return(NULL); - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start; - ret->index = startindex; - ret->user2 = end; - ret->index2 = endindex; + ret = xmlXPtrNewRangeInternal(start, startindex, end, endindex); xmlXPtrRangeCheckOrder(ret); return(ret); } @@ -381,17 +410,8 @@ xmlXPtrNewRangePoints(xmlXPathObjectPtr start, xmlXPathObjectPtr end) { if (end->type != XPATH_POINT) return(NULL); - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start->user; - ret->index = start->index; - ret->user2 = end->user; - ret->index2 = end->index; + ret = xmlXPtrNewRangeInternal(start->user, start->index, end->user, + end->index); xmlXPtrRangeCheckOrder(ret); return(ret); } @@ -416,17 +436,7 @@ xmlXPtrNewRangePointNode(xmlXPathObjectPtr start, xmlNodePtr end) { if (start->type != XPATH_POINT) return(NULL); - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start->user; - ret->index = start->index; - ret->user2 = end; - ret->index2 = -1; + ret = xmlXPtrNewRangeInternal(start->user, start->index, end, -1); xmlXPtrRangeCheckOrder(ret); return(ret); } @@ -453,17 +463,7 @@ xmlXPtrNewRangeNodePoint(xmlNodePtr start, xmlXPathObjectPtr end) { if (end->type != XPATH_POINT) return(NULL); - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start; - ret->index = -1; - ret->user2 = end->user; - ret->index2 = end->index; + ret = xmlXPtrNewRangeInternal(start, -1, end->user, end->index); xmlXPtrRangeCheckOrder(ret); return(ret); } @@ -486,17 +486,7 @@ xmlXPtrNewRangeNodes(xmlNodePtr start, xmlNodePtr end) { if (end == NULL) return(NULL); - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start; - ret->index = -1; - ret->user2 = end; - ret->index2 = -1; + ret = xmlXPtrNewRangeInternal(start, -1, end, -1); xmlXPtrRangeCheckOrder(ret); return(ret); } @@ -516,17 +506,7 @@ xmlXPtrNewCollapsedRange(xmlNodePtr start) { if (start == NULL) return(NULL); - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start; - ret->index = -1; - ret->user2 = NULL; - ret->index2 = -1; + ret = xmlXPtrNewRangeInternal(start, -1, NULL, -1); return(ret); } @@ -541,6 +521,8 @@ xmlXPtrNewCollapsedRange(xmlNodePtr start) { */ xmlXPathObjectPtr xmlXPtrNewRangeNodeObject(xmlNodePtr start, xmlXPathObjectPtr end) { + xmlNodePtr endNode; + int endIndex; xmlXPathObjectPtr ret; if (start == NULL) @@ -549,7 +531,12 @@ xmlXPtrNewRangeNodeObject(xmlNodePtr start, xmlXPathObjectPtr end) { return(NULL); switch (end->type) { case XPATH_POINT: + endNode = end->user; + endIndex = end->index; + break; case XPATH_RANGE: + endNode = end->user2; + endIndex = end->index2; break; case XPATH_NODESET: /* @@ -557,39 +544,15 @@ xmlXPtrNewRangeNodeObject(xmlNodePtr start, xmlXPathObjectPtr end) { */ if (end->nodesetval->nodeNr <= 0) return(NULL); + endNode = end->nodesetval->nodeTab[end->nodesetval->nodeNr - 1]; + endIndex = -1; break; default: /* TODO */ return(NULL); } - ret = (xmlXPathObjectPtr) xmlMalloc(sizeof(xmlXPathObject)); - if (ret == NULL) { - xmlXPtrErrMemory("allocating range"); - return(NULL); - } - memset(ret, 0 , (size_t) sizeof(xmlXPathObject)); - ret->type = XPATH_RANGE; - ret->user = start; - ret->index = -1; - switch (end->type) { - case XPATH_POINT: - ret->user2 = end->user; - ret->index2 = end->index; - break; - case XPATH_RANGE: - ret->user2 = end->user2; - ret->index2 = end->index2; - break; - case XPATH_NODESET: { - ret->user2 = end->nodesetval->nodeTab[end->nodesetval->nodeNr - 1]; - ret->index2 = -1; - break; - } - default: - STRANGE - return(NULL); - } + ret = xmlXPtrNewRangeInternal(start, -1, endNode, endIndex); xmlXPtrRangeCheckOrder(ret); return(ret); } -- GitLab