[xml/sgml-pkgs] Bug#774358: libxml2: CVE-2014-3660 patch makes installation-guide FTBFS
Salvatore Bonaccorso
carnil at debian.org
Sat Apr 4 10:52:46 UTC 2015
On Sat, Apr 04, 2015 at 11:14:24AM +0200, Salvatore Bonaccorso wrote:
> Hi Cyril, hi Samuel,
>
> On Fri, Apr 03, 2015 at 11:34:06PM +0200, Cyril Brulebois wrote:
> > Hi people,
> >
> > (adding debian-boot@ for reference.)
> >
> > Samuel Thibault <sthibault at debian.org> (2015-03-26):
> > > Samuel Thibault, le Thu 26 Mar 2015 02:17:01 +0100, a ?crit :
> > > > Control: found -1 2.8.0+dfsg1-7+wheezy3
> > > >
> > > > This is still an issue in stable, the proposed patch was not applied
> > > > there, and thus installation-guide still FTBFS on wheezy, notably on our
> > > > dillon.debian.org machine, thus making http://d-i.debian.org/manual/
> > > > completely out of date. Could this be proposed for stable update?
> > > >
> > > > I have attached the proposed patch again.
> > >
> > > Just to insist: while the symptoms of my report (#774358) may look like
> > > #768089, the *actual* bug is *not* the same. Please read my bug report
> > > and the proposed patch again: the issue is that the security fix for
> > > CVE-2014-3660 from a newer version of libxml2 (2.9.x) was backported
> > > into the libxml2 of wheezy (2.8.x) without noticing the subtle source
> > > code difference which does matter a lot.
> >
> > As one of the guys receiving a notification of the FTBFS every time
> > the crontab entry is triggered, and who would like to make sure the
> > installation guide is actually buildable *and* up-to-date, I really
> > would like to get a fix for this regression ASAP. It's been more than
> > 3 months since this bug report about ***stable being broken*** has
> > been opened.
> >
> > Thanks already.
>
> I prepared an update adding the two additional commits which seem
> required as basis for the patch for CVE-2014-3660. I have uploaded it
> here:
>
> https://people.debian.org/~carnil/tmp/libxml2/
For reference, attached as well the debdiff.
Regards,
Salvatore
-------------- next part --------------
diff -Nru libxml2-2.8.0+dfsg1/debian/changelog libxml2-2.8.0+dfsg1/debian/changelog
--- libxml2-2.8.0+dfsg1/debian/changelog 2015-02-04 20:12:17.000000000 +0100
+++ libxml2-2.8.0+dfsg1/debian/changelog 2015-04-04 11:01:39.000000000 +0200
@@ -1,3 +1,18 @@
+libxml2 (2.8.0+dfsg1-7+wheezy4) wheezy-security; urgency=high
+
+ * Non-maintainer upload by the Security Team.
+ * Add missing required patches for CVE-2014-3660.
+ The two upstream commits a3f1e3e5712257fd279917a9158278534e8f4b72 and
+ cff2546f13503ac028e4c1f63c7b6d85f2f2d777 are required in addition to the
+ commit be2a7edaf289c5da74a4f9ed3a0b6c733e775230 to fix CVE-2014-3660 due
+ to changes in the use of ent->checked.
+ Fixes "libxml2: CVE-2014-3660 patch makes installation-guide FTBFS".
+ (Closes: #774358)
+ * Refresh cve-2014-3660.patch patch
+ * Refresh cve-2014-3660-bis.patch patch
+
+ -- Salvatore Bonaccorso <carnil at debian.org> Sat, 04 Apr 2015 11:01:18 +0200
+
libxml2 (2.8.0+dfsg1-7+wheezy3) wheezy-security; urgency=high
* Do not fetch external parsed entities unless asked to do so. This
diff -Nru libxml2-2.8.0+dfsg1/debian/patches/0001-Avoid-extra-processing-on-entities.patch libxml2-2.8.0+dfsg1/debian/patches/0001-Avoid-extra-processing-on-entities.patch
--- libxml2-2.8.0+dfsg1/debian/patches/0001-Avoid-extra-processing-on-entities.patch 1970-01-01 01:00:00.000000000 +0100
+++ libxml2-2.8.0+dfsg1/debian/patches/0001-Avoid-extra-processing-on-entities.patch 2015-04-04 11:01:39.000000000 +0200
@@ -0,0 +1,62 @@
+From a3f1e3e5712257fd279917a9158278534e8f4b72 Mon Sep 17 00:00:00 2001
+From: Daniel Veillard <veillard at redhat.com>
+Date: Mon, 11 Mar 2013 13:57:53 +0800
+Subject: [PATCH] Avoid extra processing on entities
+
+If an entity has already been checked for correctness no
+need to check it on every reference
+---
+ SAX2.c | 6 ++++--
+ parser.c | 8 ++++++--
+ result/att11.sax | 2 --
+ result/att11.sax2 | 2 --
+ 4 files changed, 10 insertions(+), 8 deletions(-)
+
+--- a/SAX2.c
++++ b/SAX2.c
+@@ -574,6 +574,7 @@ xmlSAX2GetEntity(void *ctx, const xmlCha
+ * parse the external entity
+ */
+ xmlNodePtr children;
++ unsigned long oldnbent = ctxt->nbentities;
+
+ val = xmlParseCtxtExternalEntity(ctxt, ret->URI,
+ ret->ExternalID, &children);
+@@ -586,8 +587,9 @@ xmlSAX2GetEntity(void *ctx, const xmlCha
+ return(NULL);
+ }
+ ret->owner = 1;
+- if (ret->checked == 0)
+- ret->checked = 1;
++ if (ret->checked == 0) {
++ ret->checked = ctxt->nbentities - oldnbent + 1;
++ }
+ }
+ return(ret);
+ }
+--- a/parser.c
++++ b/parser.c
+@@ -3953,9 +3953,13 @@ xmlParseAttValueComplex(xmlParserCtxtPtr
+ * entities problems
+ */
+ if ((ent->etype != XML_INTERNAL_PREDEFINED_ENTITY) &&
+- (ent->content != NULL)) {
++ (ent->content != NULL) && (ent->checked == 0)) {
++ unsigned long oldnbent = ctxt->nbentities;
++
+ rep = xmlStringDecodeEntities(ctxt, ent->content,
+ XML_SUBSTITUTE_REF, 0, 0, 0);
++
++ ent->checked = ctxt->nbentities - oldnbent + 1;
+ if (rep != NULL) {
+ xmlFree(rep);
+ rep = NULL;
+@@ -7074,7 +7078,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
+ * Store the number of entities needing parsing for this entity
+ * content and do checkings
+ */
+- ent->checked = ctxt->nbentities - oldnbent;
++ ent->checked = ctxt->nbentities - oldnbent + 1;
+ if (ret == XML_ERR_ENTITY_LOOP) {
+ xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
+ xmlFreeNodeList(list);
diff -Nru libxml2-2.8.0+dfsg1/debian/patches/0001-Cache-presence-of-in-entities-content.patch libxml2-2.8.0+dfsg1/debian/patches/0001-Cache-presence-of-in-entities-content.patch
--- libxml2-2.8.0+dfsg1/debian/patches/0001-Cache-presence-of-in-entities-content.patch 1970-01-01 01:00:00.000000000 +0100
+++ libxml2-2.8.0+dfsg1/debian/patches/0001-Cache-presence-of-in-entities-content.patch 2015-04-04 11:01:39.000000000 +0200
@@ -0,0 +1,121 @@
+From cff2546f13503ac028e4c1f63c7b6d85f2f2d777 Mon Sep 17 00:00:00 2001
+From: Daniel Veillard <veillard at redhat.com>
+Date: Mon, 11 Mar 2013 15:57:55 +0800
+Subject: [PATCH] Cache presence of '<' in entities content
+
+slightly modify how ent->checked is used, and use the lowest bit to
+keep the information
+---
+ SAX2.c | 4 +++-
+ include/libxml/entities.h | 3 ++-
+ parser.c | 30 ++++++++++++++++++------------
+ 3 files changed, 23 insertions(+), 14 deletions(-)
+
+--- a/SAX2.c
++++ b/SAX2.c
+@@ -588,7 +588,9 @@ xmlSAX2GetEntity(void *ctx, const xmlCha
+ }
+ ret->owner = 1;
+ if (ret->checked == 0) {
+- ret->checked = ctxt->nbentities - oldnbent + 1;
++ ret->checked = (ctxt->nbentities - oldnbent + 1) * 2;
++ if ((ret->content != NULL) && (xmlStrchr(ret->content, '<')))
++ ret->checked |= 1;
+ }
+ }
+ return(ret);
+--- a/include/libxml/entities.h
++++ b/include/libxml/entities.h
+@@ -58,7 +58,8 @@ struct _xmlEntity {
+ int owner; /* does the entity own the childrens */
+ int checked; /* was the entity content checked */
+ /* this is also used to count entites
+- * references done from that entity */
++ * references done from that entity
++ * and if it contains '<' */
+ };
+
+ /*
+--- a/parser.c
++++ b/parser.c
+@@ -167,7 +167,7 @@ xmlParserEntityCheck(xmlParserCtxtPtr ct
+ /*
+ * use the number of parsed entities in the replacement
+ */
+- size = ent->checked;
++ size = ent->checked / 2;
+
+ /*
+ * The amount of data parsed counting entities size only once
+@@ -2724,7 +2724,7 @@ xmlStringLenDecodeEntities(xmlParserCtxt
+ (ctxt->lastError.code == XML_ERR_INTERNAL_ERROR))
+ goto int_error;
+ if (ent != NULL)
+- ctxt->nbentities += ent->checked;
++ ctxt->nbentities += ent->checked / 2;
+ if ((ent != NULL) &&
+ (ent->etype == XML_INTERNAL_PREDEFINED_ENTITY)) {
+ if (ent->content != NULL) {
+@@ -2775,7 +2775,7 @@ xmlStringLenDecodeEntities(xmlParserCtxt
+ if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
+ goto int_error;
+ if (ent != NULL)
+- ctxt->nbentities += ent->checked;
++ ctxt->nbentities += ent->checked / 2;
+ if (ent != NULL) {
+ if (ent->content == NULL) {
+ xmlLoadEntityContent(ctxt, ent);
+@@ -3959,8 +3959,10 @@ xmlParseAttValueComplex(xmlParserCtxtPtr
+ rep = xmlStringDecodeEntities(ctxt, ent->content,
+ XML_SUBSTITUTE_REF, 0, 0, 0);
+
+- ent->checked = ctxt->nbentities - oldnbent + 1;
++ ent->checked = (ctxt->nbentities - oldnbent + 1) * 2;
+ if (rep != NULL) {
++ if (xmlStrchr(rep, '<'))
++ ent->checked |= 1;
+ xmlFree(rep);
+ rep = NULL;
+ }
+@@ -7078,7 +7080,9 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
+ * Store the number of entities needing parsing for this entity
+ * content and do checkings
+ */
+- ent->checked = ctxt->nbentities - oldnbent + 1;
++ ent->checked = (ctxt->nbentities - oldnbent + 1) * 2;
++ if ((ent->content != NULL) && (xmlStrchr(ent->content, '<')))
++ ent->checked |= 1;
+ if (ret == XML_ERR_ENTITY_LOOP) {
+ xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
+ xmlFreeNodeList(list);
+@@ -7143,9 +7147,9 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
+ list = NULL;
+ }
+ if (ent->checked == 0)
+- ent->checked = 1;
++ ent->checked = 2;
+ } else if (ent->checked != 1) {
+- ctxt->nbentities += ent->checked;
++ ctxt->nbentities += ent->checked / 2;
+ }
+
+ /*
+@@ -7508,11 +7512,13 @@ xmlParseEntityRef(xmlParserCtxtPtr ctxt)
+ * not contain a <.
+ */
+ else if ((ctxt->instate == XML_PARSER_ATTRIBUTE_VALUE) &&
+- (ent != NULL) && (ent->content != NULL) &&
+- (ent->etype != XML_INTERNAL_PREDEFINED_ENTITY) &&
+- (xmlStrchr(ent->content, '<'))) {
+- xmlFatalErrMsgStr(ctxt, XML_ERR_LT_IN_ATTRIBUTE,
+- "'<' in entity '%s' is not allowed in attributes values\n", name);
++ (ent != NULL) &&
++ (ent->etype != XML_INTERNAL_PREDEFINED_ENTITY)) {
++ if ((ent->checked & 1) || ((ent->checked == 0) &&
++ (ent->content != NULL) &&(xmlStrchr(ent->content, '<')))) {
++ xmlFatalErrMsgStr(ctxt, XML_ERR_LT_IN_ATTRIBUTE,
++ "'<' in entity '%s' is not allowed in attributes values\n", name);
++ }
+ }
+
+ /*
diff -Nru libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660-bis.patch libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660-bis.patch
--- libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660-bis.patch 2015-02-04 20:12:17.000000000 +0100
+++ libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660-bis.patch 2015-04-04 11:01:39.000000000 +0200
@@ -11,7 +11,7 @@
--- a/parser.c
+++ b/parser.c
-@@ -7059,7 +7059,8 @@
+@@ -7065,7 +7065,8 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
* far more secure as the parser will only process data coming from
* the document entity by default.
*/
diff -Nru libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660.patch libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660.patch
--- libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660.patch 2015-02-04 20:12:17.000000000 +0100
+++ libxml2-2.8.0+dfsg1/debian/patches/cve-2014-3660.patch 2015-04-04 11:01:39.000000000 +0200
@@ -6,11 +6,9 @@
parser.c | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
-diff --git a/parser.c b/parser.c
-index 7ef712d..b435913 100644
--- a/parser.c
+++ b/parser.c
-@@ -127,6 +127,29 @@ xmlParserEntityCheck(xmlParserCtxtPtr ctxt, size_t size,
+@@ -127,6 +127,29 @@ xmlParserEntityCheck(xmlParserCtxtPtr ct
return (0);
if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
return (1);
@@ -40,7 +38,7 @@
if (replacement != 0) {
if (replacement < XML_MAX_TEXT_LENGTH)
return(0);
-@@ -186,9 +209,12 @@ xmlParserEntityCheck(xmlParserCtxtPtr ctxt, size_t size,
+@@ -186,9 +209,12 @@ xmlParserEntityCheck(xmlParserCtxtPtr ct
return (0);
} else {
/*
@@ -55,7 +53,7 @@
}
xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
return (1);
-@@ -2553,6 +2579,7 @@ xmlParserHandlePEReference(xmlParserCtxtPtr ctxt) {
+@@ -2553,6 +2579,7 @@ xmlParserHandlePEReference(xmlParserCtxt
name, NULL);
ctxt->valid = 0;
}
@@ -63,23 +61,23 @@
} else if (ctxt->input->free != deallocblankswrapper) {
input = xmlNewBlanksWrapperInputStream(ctxt, entity);
if (xmlPushInput(ctxt, input) < 0)
-@@ -2723,6 +2750,7 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
+@@ -2723,6 +2750,7 @@ xmlStringLenDecodeEntities(xmlParserCtxt
if ((ctxt->lastError.code == XML_ERR_ENTITY_LOOP) ||
(ctxt->lastError.code == XML_ERR_INTERNAL_ERROR))
goto int_error;
+ xmlParserEntityCheck(ctxt, 0, ent, 0);
if (ent != NULL)
- ctxt->nbentities += ent->checked;
+ ctxt->nbentities += ent->checked / 2;
if ((ent != NULL) &&
-@@ -2774,6 +2802,7 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
+@@ -2774,6 +2802,7 @@ xmlStringLenDecodeEntities(xmlParserCtxt
ent = xmlParseStringPEReference(ctxt, &str);
if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
goto int_error;
+ xmlParserEntityCheck(ctxt, 0, ent, 0);
if (ent != NULL)
- ctxt->nbentities += ent->checked;
+ ctxt->nbentities += ent->checked / 2;
if (ent != NULL) {
-@@ -7127,6 +7156,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
+@@ -7142,6 +7171,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
(ret != XML_WAR_UNDECLARED_ENTITY)) {
xmlFatalErrMsgStr(ctxt, XML_ERR_UNDECLARED_ENTITY,
"Entity '%s' failed to parse\n", ent->name);
@@ -87,7 +85,7 @@
} else if (list != NULL) {
xmlFreeNodeList(list);
list = NULL;
-@@ -7235,7 +7265,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
+@@ -7250,7 +7280,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
/*
* We are copying here, make sure there is no abuse
*/
@@ -96,7 +94,7 @@
if (xmlParserEntityCheck(ctxt, 0, ent, ctxt->sizeentcopy))
return;
-@@ -7283,7 +7313,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
+@@ -7298,7 +7328,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
/*
* We are copying here, make sure there is no abuse
*/
@@ -105,7 +103,7 @@
if (xmlParserEntityCheck(ctxt, 0, ent, ctxt->sizeentcopy))
return;
-@@ -7467,6 +7497,7 @@ xmlParseEntityRef(xmlParserCtxtPtr ctxt) {
+@@ -7482,6 +7512,7 @@ xmlParseEntityRef(xmlParserCtxtPtr ctxt)
ctxt->sax->reference(ctxt->userData, name);
}
}
@@ -113,7 +111,7 @@
ctxt->valid = 0;
}
-@@ -7654,6 +7685,7 @@ xmlParseStringEntityRef(xmlParserCtxtPtr ctxt, const xmlChar ** str) {
+@@ -7671,6 +7702,7 @@ xmlParseStringEntityRef(xmlParserCtxtPtr
"Entity '%s' not defined\n",
name);
}
@@ -121,7 +119,7 @@
/* TODO ? check regressions ctxt->valid = 0; */
}
-@@ -7812,6 +7844,7 @@ xmlParsePEReference(xmlParserCtxtPtr ctxt)
+@@ -7829,6 +7861,7 @@ xmlParsePEReference(xmlParserCtxtPtr ctx
name, NULL);
ctxt->valid = 0;
}
@@ -129,7 +127,7 @@
} else {
/*
* Internal checking in case the entity quest barfed
-@@ -8039,6 +8072,7 @@ xmlParseStringPEReference(xmlParserCtxtPtr ctxt, const xmlChar **str) {
+@@ -8056,6 +8089,7 @@ xmlParseStringPEReference(xmlParserCtxtP
name, NULL);
ctxt->valid = 0;
}
diff -Nru libxml2-2.8.0+dfsg1/debian/patches/series libxml2-2.8.0+dfsg1/debian/patches/series
--- libxml2-2.8.0+dfsg1/debian/patches/series 2015-02-04 20:12:17.000000000 +0100
+++ libxml2-2.8.0+dfsg1/debian/patches/series 2015-04-04 11:01:39.000000000 +0200
@@ -10,5 +10,7 @@
cve-2013-2877.patch
cve-2014-0191.patch
cve-2014-0191-bis.patch
+0001-Avoid-extra-processing-on-entities.patch
+0001-Cache-presence-of-in-entities-content.patch
cve-2014-3660.patch
cve-2014-3660-bis.patch
More information about the debian-xml-sgml-pkgs
mailing list