Merge pull request #6835 from micmac1/xml2-cve-17.01

libxml2: add Debian patches to address CVEs
This commit is contained in:
Michael Heimpold 2018-08-21 21:11:38 +02:00 committed by GitHub
commit 1553fad25f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 1150 additions and 1 deletions

View File

@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
PKG_NAME:=libxml2
PKG_VERSION:=2.9.4
PKG_RELEASE:=1
PKG_RELEASE:=2
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
PKG_SOURCE_URL:=http://gd.tuwien.ac.at/languages/libxml/ \

View File

@ -0,0 +1,53 @@
From: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Sat, 25 Jun 2016 12:35:50 +0200
Subject: Fix NULL pointer deref in XPointer range-to
- Check for errors after evaluating first operand.
- Add sanity check for empty stack.
Found with afl-fuzz.
---
result/XPath/xptr/viderror | 4 ++++
test/XPath/xptr/viderror | 1 +
xpath.c | 7 ++++++-
3 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 result/XPath/xptr/viderror
create mode 100644 test/XPath/xptr/viderror
diff --git a/result/XPath/xptr/viderror b/result/XPath/xptr/viderror
new file mode 100644
index 0000000..d589882
--- /dev/null
+++ b/result/XPath/xptr/viderror
@@ -0,0 +1,4 @@
+
+========================
+Expression: xpointer(non-existing-fn()/range-to(id('chapter2')))
+Object is empty (NULL)
diff --git a/test/XPath/xptr/viderror b/test/XPath/xptr/viderror
new file mode 100644
index 0000000..da8c53b
--- /dev/null
+++ b/test/XPath/xptr/viderror
@@ -0,0 +1 @@
+xpointer(non-existing-fn()/range-to(id('chapter2')))
diff --git a/xpath.c b/xpath.c
index 113bce6..751665b 100644
--- a/xpath.c
+++ b/xpath.c
@@ -14005,9 +14005,14 @@ xmlXPathCompOpEval(xmlXPathParserContextPtr ctxt, xmlXPathStepOpPtr op)
xmlNodeSetPtr oldset;
int i, j;
- if (op->ch1 != -1)
+ if (op->ch1 != -1) {
total +=
xmlXPathCompOpEval(ctxt, &comp->steps[op->ch1]);
+ CHECK_ERROR0;
+ }
+ if (ctxt->value == NULL) {
+ XP_ERROR0(XPATH_INVALID_OPERAND);
+ }
if (op->ch2 == -1)
return (total);

View File

@ -0,0 +1,34 @@
From a005199330b86dada19d162cae15ef9bdcb6baa8 Mon Sep 17 00:00:00 2001
From: Nick Wellnhofer <wellnhofer@aevum.de>
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 751665b..d992841 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.10.1

View File

@ -0,0 +1,138 @@
From 9ab01a277d71f54d3143c2cf333c5c2e9aaedd9e Mon Sep 17 00:00:00 2001
From: Nick Wellnhofer <wellnhofer@aevum.de>
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(-)
--- a/xpath.c
+++ b/xpath.c
@@ -10691,13 +10691,18 @@ xmlXPathCompPathExpr(xmlXPathParserConte
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,
--- a/xpointer.c
+++ b/xpointer.c
@@ -1332,8 +1332,6 @@ xmlXPtrNewContext(xmlDocPtr doc, xmlNode
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(xmlXPathParse
* @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);
}
/**

View File

@ -0,0 +1,249 @@
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 a7b03fb..694d120 100644
--- a/xpointer.c
+++ b/xpointer.c
@@ -320,6 +320,45 @@ xmlXPtrRangesEqual(xmlXPathObjectPtr range1, xmlXPathObjectPtr range2) {
}
/**
+ * 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
* @startindex: the start index
@@ -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);
}
--
2.10.1

View File

@ -0,0 +1,50 @@
From e905f08123e4a6e7731549e6f09dadff4cab65bd Mon Sep 17 00:00:00 2001
From: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Sun, 26 Jun 2016 12:38:28 +0200
Subject: [PATCH] Fix more NULL pointer derefs in xpointer.c
Found with afl-fuzz.
---
xpointer.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/xpointer.c b/xpointer.c
index 694d120..e643ee9 100644
--- a/xpointer.c
+++ b/xpointer.c
@@ -542,7 +542,7 @@ xmlXPtrNewRangeNodeObject(xmlNodePtr start, xmlXPathObjectPtr end) {
/*
* Empty set ...
*/
- if (end->nodesetval->nodeNr <= 0)
+ if ((end->nodesetval == NULL) || (end->nodesetval->nodeNr <= 0))
return(NULL);
endNode = end->nodesetval->nodeTab[end->nodesetval->nodeNr - 1];
endIndex = -1;
@@ -1361,7 +1361,7 @@ xmlXPtrEval(const xmlChar *str, xmlXPathContextPtr ctx) {
*/
xmlNodeSetPtr set;
set = tmp->nodesetval;
- if ((set->nodeNr != 1) ||
+ if ((set == NULL) || (set->nodeNr != 1) ||
(set->nodeTab[0] != (xmlNodePtr) ctx->doc))
stack++;
} else
@@ -2034,9 +2034,11 @@ xmlXPtrRangeFunction(xmlXPathParserContextPtr ctxt, int nargs) {
xmlXPathFreeObject(set);
XP_ERROR(XPATH_MEMORY_ERROR);
}
- for (i = 0;i < oldset->locNr;i++) {
- xmlXPtrLocationSetAdd(newset,
- xmlXPtrCoveringRange(ctxt, oldset->locTab[i]));
+ if (oldset != NULL) {
+ for (i = 0;i < oldset->locNr;i++) {
+ xmlXPtrLocationSetAdd(newset,
+ xmlXPtrCoveringRange(ctxt, oldset->locTab[i]));
+ }
}
/*
--
2.1.4

View File

@ -0,0 +1,66 @@
From 256366ed60f8795279b25f7b7b55e8089b4c6ff4 Mon Sep 17 00:00:00 2001
From: Alex Henrie <alexhenrie24@gmail.com>
Date: Thu, 26 May 2016 17:38:35 -0600
Subject: [PATCH] Fix attribute decoding during XML schema validation
For https://bugzilla.gnome.org/show_bug.cgi?id=766834
vctxt->parserCtxt is always NULL in xmlSchemaSAXHandleStartElementNs,
so this function can't call xmlStringLenDecodeEntities to decode the
entities.
---
xmlschemas.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/xmlschemas.c b/xmlschemas.c
index e1b3a4f..59535e5 100644
--- a/xmlschemas.c
+++ b/xmlschemas.c
@@ -27391,6 +27391,7 @@ xmlSchemaSAXHandleStartElementNs(void *ctx,
* attributes yet.
*/
if (nb_attributes != 0) {
+ int valueLen, k, l;
xmlChar *value;
for (j = 0, i = 0; i < nb_attributes; i++, j += 5) {
@@ -27400,12 +27401,31 @@ xmlSchemaSAXHandleStartElementNs(void *ctx,
* libxml2 differs from normal SAX here in that it escapes all ampersands
* as &#38; instead of delivering the raw converted string. Changing the
* behavior at this point would break applications that use this API, so
- * we are forced to work around it. There is no danger of accidentally
- * decoding some entity other than &#38; in this step because without
- * unescaped ampersands there can be no other entities in the string.
+ * we are forced to work around it.
*/
- value = xmlStringLenDecodeEntities(vctxt->parserCtxt, attributes[j+3],
- attributes[j+4] - attributes[j+3], XML_SUBSTITUTE_REF, 0, 0, 0);
+ valueLen = attributes[j+4] - attributes[j+3];
+ value = xmlMallocAtomic(valueLen + 1);
+ if (value == NULL) {
+ xmlSchemaVErrMemory(vctxt,
+ "allocating string for decoded attribute",
+ NULL);
+ goto internal_error;
+ }
+ for (k = 0, l = 0; k < valueLen; l++) {
+ if (k < valueLen - 4 &&
+ attributes[j+3][k+0] == '&' &&
+ attributes[j+3][k+1] == '#' &&
+ attributes[j+3][k+2] == '3' &&
+ attributes[j+3][k+3] == '8' &&
+ attributes[j+3][k+4] == ';') {
+ value[l] = '&';
+ k += 5;
+ } else {
+ value[l] = attributes[j+3][k];
+ k++;
+ }
+ }
+ value[l] = '\0';
/*
* TODO: Set the node line.
*/
--
2.8.3

View File

@ -0,0 +1,34 @@
From: Daniel Veillard <veillard@redhat.com>
Date: Fri, 7 Apr 2017 17:13:28 +0200
Subject: Increase buffer space for port in HTTP redirect support
Origin: https://git.gnome.org/browse/libxml2/commit/?id=5dca9eea1bd4263bfa4d037ab2443de1cd730f7e
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=780690
Bug-Debian: https://bugs.debian.org/870865
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-7376
For https://bugzilla.gnome.org/show_bug.cgi?id=780690
nanohttp.c: the code wrongly assumed a short int port value.
---
nanohttp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/nanohttp.c b/nanohttp.c
index e109ad75..373425de 100644
--- a/nanohttp.c
+++ b/nanohttp.c
@@ -1423,9 +1423,9 @@ retry:
if (ctxt->port != 80) {
/* reserve space for ':xxxxx', incl. potential proxy */
if (proxy)
- blen += 12;
+ blen += 17;
else
- blen += 6;
+ blen += 11;
}
bp = (char*)xmlMallocAtomic(blen);
if ( bp == NULL ) {
--
2.11.0

View File

@ -0,0 +1,38 @@
From: Neel Mehta <nmehta@google.com>
Date: Fri, 7 Apr 2017 17:43:02 +0200
Subject: Prevent unwanted external entity reference
Origin: https://git.gnome.org/browse/libxml2/commit/?id=90ccb58242866b0ba3edbef8fe44214a101c2b3e
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=780691
Bug-Debian: https://bugs.debian.org/870867
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-7375
For https://bugzilla.gnome.org/show_bug.cgi?id=780691
* parser.c: add a specific check to avoid PE reference
---
parser.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/parser.c b/parser.c
index 609a2703..c2c812de 100644
--- a/parser.c
+++ b/parser.c
@@ -8123,6 +8123,15 @@ xmlParsePEReference(xmlParserCtxtPtr ctxt)
if (xmlPushInput(ctxt, input) < 0)
return;
} else {
+ if ((entity->etype == XML_EXTERNAL_PARAMETER_ENTITY) &&
+ ((ctxt->options & XML_PARSE_NOENT) == 0) &&
+ ((ctxt->options & XML_PARSE_DTDVALID) == 0) &&
+ ((ctxt->options & XML_PARSE_DTDLOAD) == 0) &&
+ ((ctxt->options & XML_PARSE_DTDATTR) == 0) &&
+ (ctxt->replaceEntities == 0) &&
+ (ctxt->validate == 0))
+ return;
+
/*
* TODO !!!
* handle the extra spaces added before and after
--
2.11.0

View File

@ -0,0 +1,280 @@
From: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Mon, 5 Jun 2017 15:37:17 +0200
Subject: Fix handling of parameter-entity references
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Origin: https://git.gnome.org/browse/libxml2/commit/?id=e26630548e7d138d2c560844c43820b6767251e3
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=781205
Bug-Debian: https://bugs.debian.org/863019
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-9049
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=781361
Bug-Debian: https://bugs.debian.org/863018
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-9050
There were two bugs where parameter-entity references could lead to an
unexpected change of the input buffer in xmlParseNameComplex and
xmlDictLookup being called with an invalid pointer.
Percent sign in DTD Names
=========================
The NEXTL macro used to call xmlParserHandlePEReference. When parsing
"complex" names inside the DTD, this could result in entity expansion
which created a new input buffer. The fix is to simply remove the call
to xmlParserHandlePEReference from the NEXTL macro. This is safe because
no users of the macro require expansion of parameter entities.
- xmlParseNameComplex
- xmlParseNCNameComplex
- xmlParseNmtoken
The percent sign is not allowed in names, which are grammatical tokens.
- xmlParseEntityValue
Parameter-entity references in entity values are expanded but this
happens in a separate step in this function.
- xmlParseSystemLiteral
Parameter-entity references are ignored in the system literal.
- xmlParseAttValueComplex
- xmlParseCharDataComplex
- xmlParseCommentComplex
- xmlParsePI
- xmlParseCDSect
Parameter-entity references are ignored outside the DTD.
- xmlLoadEntityContent
This function is only called from xmlStringLenDecodeEntities and
entities are replaced in a separate step immediately after the function
call.
This bug could also be triggered with an internal subset and double
entity expansion.
This fixes bug 766956 initially reported by Wei Lei and independently by
Chromium's ClusterFuzz, Hanno Böck, and Marco Grassi. Thanks to everyone
involved.
xmlParseNameComplex with XML_PARSE_OLD10
========================================
When parsing Names inside an expanded parameter entity with the
XML_PARSE_OLD10 option, xmlParseNameComplex would call xmlGROW via the
GROW macro if the input buffer was exhausted. At the end of the
parameter entity's replacement text, this function would then call
xmlPopInput which invalidated the input buffer.
There should be no need to invoke GROW in this situation because the
buffer is grown periodically every XML_PARSER_CHUNK_SIZE characters and,
at least for UTF-8, in xmlCurrentChar. This also matches the code path
executed when XML_PARSE_OLD10 is not set.
This fixes bugs 781205 (CVE-2017-9049) and 781361 (CVE-2017-9050).
Thanks to Marcel Böhme and Thuan Pham for the report.
Additional hardening
====================
A separate check was added in xmlParseNameComplex to validate the
buffer size.
---
Makefile.am | 18 ++++++++++++++++++
parser.c | 18 ++++++++++--------
result/errors10/781205.xml | 0
result/errors10/781205.xml.err | 21 +++++++++++++++++++++
result/errors10/781361.xml | 0
result/errors10/781361.xml.err | 13 +++++++++++++
result/valid/766956.xml | 0
result/valid/766956.xml.err | 9 +++++++++
result/valid/766956.xml.err.rdr | 10 ++++++++++
runtest.c | 3 +++
test/errors10/781205.xml | 3 +++
test/errors10/781361.xml | 3 +++
test/valid/766956.xml | 2 ++
test/valid/dtds/766956.dtd | 2 ++
14 files changed, 94 insertions(+), 8 deletions(-)
create mode 100644 result/errors10/781205.xml
create mode 100644 result/errors10/781205.xml.err
create mode 100644 result/errors10/781361.xml
create mode 100644 result/errors10/781361.xml.err
create mode 100644 result/valid/766956.xml
create mode 100644 result/valid/766956.xml.err
create mode 100644 result/valid/766956.xml.err.rdr
create mode 100644 test/errors10/781205.xml
create mode 100644 test/errors10/781361.xml
create mode 100644 test/valid/766956.xml
create mode 100644 test/valid/dtds/766956.dtd
--- a/Makefile.am
+++ b/Makefile.am
@@ -422,6 +422,24 @@ Errtests : xmllint$(EXEEXT)
if [ -n "$$log" ] ; then echo $$name result ; echo $$log ; fi ; \
rm result.$$name error.$$name ; \
fi ; fi ; done)
+ @echo "## Error cases regression tests (old 1.0)"
+ -@(for i in $(srcdir)/test/errors10/*.xml ; do \
+ name=`basename $$i`; \
+ if [ ! -d $$i ] ; then \
+ if [ ! -f $(srcdir)/result/errors10/$$name ] ; then \
+ echo New test file $$name ; \
+ $(CHECKER) $(top_builddir)/xmllint --oldxml10 $$i \
+ 2> $(srcdir)/result/errors10/$$name.err \
+ > $(srcdir)/result/errors10/$$name ; \
+ grep "MORY ALLO" .memdump | grep -v "MEMORY ALLOCATED : 0"; \
+ else \
+ log=`$(CHECKER) $(top_builddir)/xmllint --oldxml10 $$i 2> error.$$name > result.$$name ; \
+ grep "MORY ALLO" .memdump | grep -v "MEMORY ALLOCATED : 0"; \
+ diff $(srcdir)/result/errors10/$$name result.$$name ; \
+ diff $(srcdir)/result/errors10/$$name.err error.$$name` ; \
+ if [ -n "$$log" ] ; then echo $$name result ; echo "$$log" ; fi ; \
+ rm result.$$name error.$$name ; \
+ fi ; fi ; done)
@echo "## Error cases stream regression tests"
-@(for i in $(srcdir)/test/errors/*.xml ; do \
name=`basename $$i`; \
--- a/parser.c
+++ b/parser.c
@@ -2115,7 +2115,6 @@ static void xmlGROW (xmlParserCtxtPtr ct
ctxt->input->line++; ctxt->input->col = 1; \
} else ctxt->input->col++; \
ctxt->input->cur += l; \
- if (*ctxt->input->cur == '%') xmlParserHandlePEReference(ctxt); \
} while (0)
#define CUR_CHAR(l) xmlCurrentChar(ctxt, &l)
@@ -3406,13 +3405,6 @@ xmlParseNameComplex(xmlParserCtxtPtr ctx
len += l;
NEXTL(l);
c = CUR_CHAR(l);
- if (c == 0) {
- count = 0;
- GROW;
- if (ctxt->instate == XML_PARSER_EOF)
- return(NULL);
- c = CUR_CHAR(l);
- }
}
}
if ((len > XML_MAX_NAME_LENGTH) &&
@@ -3420,6 +3412,16 @@ xmlParseNameComplex(xmlParserCtxtPtr ctx
xmlFatalErr(ctxt, XML_ERR_NAME_TOO_LONG, "Name");
return(NULL);
}
+ if (ctxt->input->cur - ctxt->input->base < len) {
+ /*
+ * There were a couple of bugs where PERefs lead to to a change
+ * of the buffer. Check the buffer size to avoid passing an invalid
+ * pointer to xmlDictLookup.
+ */
+ xmlFatalErr(ctxt, XML_ERR_INTERNAL_ERROR,
+ "unexpected change of input buffer");
+ return (NULL);
+ }
if ((*ctxt->input->cur == '\n') && (ctxt->input->cur[-1] == '\r'))
return(xmlDictLookup(ctxt->dict, ctxt->input->cur - (len + 1), len));
return(xmlDictLookup(ctxt->dict, ctxt->input->cur - len, len));
--- /dev/null
+++ b/result/errors10/781205.xml.err
@@ -0,0 +1,21 @@
+Entity: line 1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
+
+ %a;
+ ^
+Entity: line 1:
+<:0000
+^
+Entity: line 1: parser error : DOCTYPE improperly terminated
+ %a;
+ ^
+Entity: line 1:
+<:0000
+^
+namespace error : Failed to parse QName ':0000'
+ %a;
+ ^
+<:0000
+ ^
+./test/errors10/781205.xml:4: parser error : Couldn't find end of Start Tag :0000 line 1
+
+^
--- /dev/null
+++ b/result/errors10/781361.xml.err
@@ -0,0 +1,13 @@
+./test/errors10/781361.xml:4: parser error : xmlParseElementDecl: 'EMPTY', 'ANY' or '(' expected
+
+^
+./test/errors10/781361.xml:4: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
+
+
+^
+./test/errors10/781361.xml:4: parser error : DOCTYPE improperly terminated
+
+^
+./test/errors10/781361.xml:4: parser error : Start tag expected, '<' not found
+
+^
--- /dev/null
+++ b/result/valid/766956.xml.err
@@ -0,0 +1,9 @@
+test/valid/dtds/766956.dtd:2: parser error : PEReference: expecting ';'
+%ä%ent;
+ ^
+Entity: line 1: parser error : Content error in the external subset
+ %ent;
+ ^
+Entity: line 1:
+value
+^
--- /dev/null
+++ b/result/valid/766956.xml.err.rdr
@@ -0,0 +1,10 @@
+test/valid/dtds/766956.dtd:2: parser error : PEReference: expecting ';'
+%ä%ent;
+ ^
+Entity: line 1: parser error : Content error in the external subset
+ %ent;
+ ^
+Entity: line 1:
+value
+^
+./test/valid/766956.xml : failed to parse
--- a/runtest.c
+++ b/runtest.c
@@ -4202,6 +4202,9 @@ testDesc testDescriptions[] = {
{ "Error cases regression tests",
errParseTest, "./test/errors/*.xml", "result/errors/", "", ".err",
0 },
+ { "Error cases regression tests (old 1.0)",
+ errParseTest, "./test/errors10/*.xml", "result/errors10/", "", ".err",
+ XML_PARSE_OLD10 },
#ifdef LIBXML_READER_ENABLED
{ "Error cases stream regression tests",
streamParseTest, "./test/errors/*.xml", "result/errors/", NULL, ".str",
--- /dev/null
+++ b/test/errors10/781205.xml
@@ -0,0 +1,3 @@
+<!DOCTYPE D [
+ <!ENTITY % a "<:0000">
+ %a;
--- /dev/null
+++ b/test/errors10/781361.xml
@@ -0,0 +1,3 @@
+<!DOCTYPE doc [
+ <!ENTITY % elem "<!ELEMENT e0000000000">
+ %elem;
--- /dev/null
+++ b/test/valid/766956.xml
@@ -0,0 +1,2 @@
+<!DOCTYPE test SYSTEM "dtds/766956.dtd">
+<test/>
--- /dev/null
+++ b/test/valid/dtds/766956.dtd
@@ -0,0 +1,2 @@
+<!ENTITY % ent "value">
+%ä%ent;

File diff suppressed because one or more lines are too long

View File

@ -0,0 +1,46 @@
From: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Tue, 6 Jun 2017 12:56:28 +0200
Subject: Fix type confusion in xmlValidateOneNamespace
Origin: https://git.gnome.org/browse/libxml2/commit/?id=92b9e8c8b3787068565a1820ba575d042f9eec66
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=780228
Bug-Debian: https://bugs.debian.org/870870
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-0663
Comment out code that casts xmlNsPtr to xmlAttrPtr. ID types on
namespace declarations make no practical sense anyway.
Fixes bug 780228.
Found with libFuzzer and ASan.
---
valid.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/valid.c b/valid.c
index 8075d3a0..c51ea290 100644
--- a/valid.c
+++ b/valid.c
@@ -4627,6 +4627,12 @@ xmlNodePtr elem, const xmlChar *prefix, xmlNsPtr ns, const xmlChar *value) {
}
}
+ /*
+ * Casting ns to xmlAttrPtr is wrong. We'd need separate functions
+ * xmlAddID and xmlAddRef for namespace declarations, but it makes
+ * no practical sense to use ID types anyway.
+ */
+#if 0
/* Validity Constraint: ID uniqueness */
if (attrDecl->atype == XML_ATTRIBUTE_ID) {
if (xmlAddID(ctxt, doc, value, (xmlAttrPtr) ns) == NULL)
@@ -4638,6 +4644,7 @@ xmlNodePtr elem, const xmlChar *prefix, xmlNsPtr ns, const xmlChar *value) {
if (xmlAddRef(ctxt, doc, value, (xmlAttrPtr) ns) == NULL)
ret = 0;
}
+#endif
/* Validity Constraint: Notation Attributes */
if (attrDecl->atype == XML_ATTRIBUTE_NOTATION) {
--
2.11.0

View File

@ -0,0 +1,39 @@
From: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Thu, 1 Jun 2017 23:12:19 +0200
Subject: Fix XPath stack frame logic
Origin: https://git.gnome.org/browse/libxml2/commit/?id=0f3b843b3534784ef57a4f9b874238aa1fda5a73
Bug-Debian: https://bugs.debian.org/883790
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=783160
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-15412
Move the calls to xmlXPathSetFrame and xmlXPathPopFrame around in
xmlXPathCompOpEvalPositionalPredicate to make sure that the context
object on the stack is actually protected. Otherwise, memory corruption
can occur when calling sloppily coded XPath extension functions.
Fixes bug 783160.
---
xpath.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xpath.c b/xpath.c
index 94815075..b816bd36 100644
--- a/xpath.c
+++ b/xpath.c
@@ -11932,11 +11932,11 @@ xmlXPathCompOpEvalPositionalPredicate(xmlXPathParserContextPtr ctxt,
}
}
- frame = xmlXPathSetFrame(ctxt);
valuePush(ctxt, contextObj);
+ frame = xmlXPathSetFrame(ctxt);
res = xmlXPathCompOpEvalToBoolean(ctxt, exprOp, 1);
- tmp = valuePop(ctxt);
xmlXPathPopFrame(ctxt, frame);
+ tmp = valuePop(ctxt);
if ((ctxt->error != XPATH_EXPRESSION_OK) || (res == -1)) {
while (tmp != contextObj) {
--
2.15.1