| Summary: | <dev-libs/libxml2-2.9.4: stack overflow in xml validator (CVE-2016-3705) | ||
|---|---|---|---|
| Product: | Gentoo Security | Reporter: | Agostino Sarubbo <ago> |
| Component: | Vulnerabilities | Assignee: | Gentoo Security <security> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | gnome |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | http://www.openwall.com/lists/oss-security/2016/05/03/4 | ||
| Whiteboard: | A2 [glsa cve] | ||
| Package list: | Runtime testing required: | --- | |
Patched via https://git.gnome.org/browse/libxml2/commit/?id=8f30bdff69edac9075f4663ce3b56b0c52d48ce6 (first released via v2.9.4). v2.9.4 landed in Gentoo repository via https://gitweb.gentoo.org/repo/gentoo.git/commit/dev-libs/libxml2?id=b68f9389191396b4febff3e7b61f939189364426 @ Security: Please vote! CVE-2016-3705 (http://nvd.nist.gov/nvd.cfm?cvename=CVE-2016-3705): The (1) xmlParserEntityCheck and (2) xmlParseAttValueComplex functions in parser.c in libxml2 2.9.3 do not properly keep track of the recursion depth, which allows context-dependent attackers to cause a denial of service (stack consumption and application crash) via a crafted XML document containing a large number of nested entity references. This issue was resolved and addressed in GLSA 201701-37 at https://security.gentoo.org/glsa/201701-37 by GLSA coordinator Thomas Deutschmann (whissi). This issue was resolved and addressed in GLSA 201701-37 at https://security.gentoo.org/glsa/201701-37 by GLSA coordinator Thomas Deutschmann (whissi). |
From ${URL} : 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 <psimons@...e.com> 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 <psimons@...e.com> 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 From 6f0af3f6b9b1c5f82a2bb5ded65923437fee5d21 Mon Sep 17 00:00:00 2001 From: Peter Simons <psimons@...e.com> Date: Fri, 15 Apr 2016 11:56:55 +0200 Subject: [PATCH 2/2] 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.8.1 From e5269fd1e83743f7e62c89eca45000c2e84e6edc Mon Sep 17 00:00:00 2001 From: Peter Simons <psimons@...e.com> Date: Thu, 14 Apr 2016 16:15:13 +0200 Subject: [PATCH 1/2] 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(-) Index: libxml2-2.9.3/tree.c =================================================================== --- libxml2-2.9.3.orig/tree.c +++ libxml2-2.9.3/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,12 @@ out: */ xmlNodePtr xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { + return xmlStringGetNodeListInternal(doc, value, 0); + } + +xmlNodePtr +xmlStringGetNodeListInternal(const xmlDoc *doc, const xmlChar *value, size_t recursionLevel) { + xmlNodePtr ret = NULL, last = NULL; xmlNodePtr node; xmlChar *val; @@ -1483,6 +1491,8 @@ xmlStringGetNodeList(const xmlDoc *doc, xmlEntityPtr ent; xmlBufPtr buf; + if (recursionLevel > 1024) return(NULL); + if (value == NULL) return(NULL); buf = xmlBufCreateSize(0); @@ -1593,8 +1603,9 @@ xmlStringGetNodeList(const xmlDoc *doc, 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) { @maintainer(s): after the bump, in case we need to stabilize the package, please let us know if it is ready for the stabilization or not.