From c1d1f7121194036608bf555f08d3062a36fd344b Mon Sep 17 00:00:00 2001

From: Nick Wellnhofer <wellnhofer@aevum.de>

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