[xml/sgml-pkgs] Bug#498768: Bug#498768: libxml2: does not correctly handle long entity names (CVE-2008-3529)
Mike Hommey
mh at glandium.org
Sat Sep 13 18:55:06 UTC 2008
On Fri, Sep 12, 2008 at 11:29:03PM -0400, Michael Gilbert wrote:
> Package: libxml2
> Version: 2.6.32.dfsg-3
> Severity: grave
> Tags: security
> Justification: user security hole
>
> ubuntu just released a fix for a problem in libxml2 [1]. the issue appears
> to currently be reserved [2], but since ubuntu has released a fix, other
> distributions need to follow suit soon to limit the window of opportunity
> for attacks. the description of the problem is
>
> It was discovered that libxml2 did not correctly handle long entity
> names. If a user were tricked into processing a specially crafted XML
> document, a remote attacker could execute arbitrary code with user
> privileges or cause the application linked against libxml2 to crash,
> leading to a denial of service.
>
> this likely affects all releases (stable, testing, and unstable).
>
> thanks for the hard work.
>
> [1] http://lwn.net/Articles/298282/
> [2] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-3529
FWIW, here is the patch.
I'm not very much convinced by the look of it...
Mike
-------------- next part --------------
* Unmerged path debian/changelog
diff --git a/include/libxml/parser.h b/include/libxml/parser.h
index 5d38f75..fa700bb 100644
--- a/include/libxml/parser.h
+++ b/include/libxml/parser.h
@@ -298,6 +298,7 @@ struct _xmlParserCtxt {
xmlError lastError;
xmlParserMode parseMode; /* the parser mode */
unsigned long nbentities; /* number of entities references */
+ unsigned long sizeentities; /* size of parsed entities */
};
/**
diff --git a/parser.c b/parser.c
index 5d7324a..8ee5ca0 100644
--- a/parser.c
+++ b/parser.c
@@ -80,6 +80,95 @@
#include <zlib.h>
#endif
+static void
+xmlFatalErr(xmlParserCtxtPtr ctxt, xmlParserErrors error, const char *info);
+
+/************************************************************************
+ * *
+ * Arbitrary limits set in the parser. *
+ * *
+ ************************************************************************/
+
+#define XML_PARSER_BIG_ENTITY 1000
+#define XML_PARSER_LOT_ENTITY 5000
+
+/*
+ * XML_PARSER_NON_LINEAR is the threshold where the ratio of parsed entity
+ * replacement over the size in byte of the input indicates that you have
+ * and eponential behaviour. A value of 10 correspond to at least 3 entity
+ * replacement per byte of input.
+ */
+#define XML_PARSER_NON_LINEAR 10
+
+/*
+ * xmlParserEntityCheck
+ *
+ * Function to check non-linear entity expansion behaviour
+ * This is here to detect and stop exponential linear entity expansion
+ * This is not a limitation of the parser but a safety
+ * boundary feature.
+ */
+static int
+xmlParserEntityCheck(xmlParserCtxtPtr ctxt, unsigned long size,
+ xmlEntityPtr ent)
+{
+ unsigned long consumed = 0;
+
+ if (ctxt == NULL)
+ return (0);
+ if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
+ return (1);
+ if (size != 0) {
+ /*
+ * Do the check based on the replacement size of the entity
+ */
+ if (size < XML_PARSER_BIG_ENTITY)
+ return(0);
+
+ /*
+ * A limit on the amount of text data reasonably used
+ */
+ if (ctxt->input != NULL) {
+ consumed = ctxt->input->consumed +
+ (ctxt->input->cur - ctxt->input->base);
+ }
+ consumed += ctxt->sizeentities;
+
+ if ((size < XML_PARSER_NON_LINEAR * consumed) &&
+ (ctxt->nbentities * 3 < XML_PARSER_NON_LINEAR * consumed))
+ return (0);
+ } else if (ent != NULL) {
+ /*
+ * use the number of parsed entities in the replacement
+ */
+ size = ent->owner;
+
+ /*
+ * The amount of data parsed counting entities size only once
+ */
+ if (ctxt->input != NULL) {
+ consumed = ctxt->input->consumed +
+ (ctxt->input->cur - ctxt->input->base);
+ }
+ consumed += ctxt->sizeentities;
+
+ /*
+ * Check the density of entities for the amount of data
+ * knowing an entity reference will take at least 3 bytes
+ */
+ if (size * 3 < consumed * XML_PARSER_NON_LINEAR)
+ return (0);
+ } else {
+ /*
+ * strange we got no data for checking just return
+ */
+ return (0);
+ }
+
+ xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
+ return (1);
+}
+
/**
* xmlParserMaxDepth:
*
@@ -2301,6 +2390,7 @@ xmlParserHandlePEReference(xmlParserCtxtPtr ctxt) {
*/
#define growBuffer(buffer) { \
xmlChar *tmp; \
+ buffer##_size += XML_PARSER_BUFFER_SIZE ; \
buffer##_size *= 2; \
tmp = (xmlChar *) \
xmlRealloc(buffer, buffer##_size * sizeof(xmlChar)); \
@@ -2344,7 +2434,7 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
return(NULL);
last = str + len;
- if ((ctxt->depth > 40) || (ctxt->nbentities >= 500000)) {
+ if (ctxt->depth > 40) {
xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
return(NULL);
}
@@ -2384,7 +2474,6 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
ent = xmlParseStringEntityRef(ctxt, &str);
if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
goto int_error;
- ctxt->nbentities++;
if (ent != NULL)
ctxt->nbentities += ent->owner;
if ((ent != NULL) &&
@@ -2409,6 +2498,10 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
buffer[nbchars++] = *current++;
if (nbchars >
buffer_size - XML_PARSER_BUFFER_SIZE) {
+ if (xmlParserEntityCheck(ctxt, nbchars, ent)) {
+ xmlFree(rep);
+ goto int_error;
+ }
growBuffer(buffer);
}
}
@@ -2434,7 +2527,6 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
ent = xmlParseStringPEReference(ctxt, &str);
if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
goto int_error;
- ctxt->nbentities++;
if (ent != NULL)
ctxt->nbentities += ent->owner;
if (ent != NULL) {
@@ -2452,6 +2544,10 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len,
buffer[nbchars++] = *current++;
if (nbchars >
buffer_size - XML_PARSER_BUFFER_SIZE) {
+ if (xmlParserEntityCheck(ctxt, nbchars, ent)) {
+ xmlFree(rep);
+ goto int_error;
+ }
growBuffer(buffer);
}
}
@@ -3355,7 +3451,7 @@ xmlParseAttValueComplex(xmlParserCtxtPtr ctxt, int *attlen, int normalize) {
* Just output the reference
*/
buf[len++] = '&';
- if (len > buf_size - i - 10) {
+ while (len > buf_size - i - 10) {
growBuffer(buf);
}
for (;i > 0;i--)
@@ -6209,11 +6305,6 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
if (ent == NULL) return;
if (!ctxt->wellFormed)
return;
- ctxt->nbentities++;
- if (ctxt->nbentities >= 500000) {
- xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
- return;
- }
was_checked = ent->checked;
if ((ent->name != NULL) &&
(ent->etype != XML_INTERNAL_PREDEFINED_ENTITY)) {
@@ -6311,6 +6402,10 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
xmlErrMsgStr(ctxt, XML_ERR_INTERNAL_ERROR,
"invalid entity type found\n", NULL);
}
+ /*
+ * Store the number of entities needing parsing for entity
+ * content and do checkings
+ */
if ((ent->owner != 0) || (ent->children == NULL)) {
ent->owner = ctxt->nbentities - oldnbent;
if (ent->owner == 0)
@@ -6318,6 +6413,15 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
}
if (ret == XML_ERR_ENTITY_LOOP) {
xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
+ xmlFreeNodeList(list);
+ return;
+ }
+ if (xmlParserEntityCheck(ctxt, 0, ent)) {
+ xmlFreeNodeList(list);
+ return;
+ }
+ if (ret == XML_ERR_ENTITY_LOOP) {
+ xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
return;
} else if ((ret == XML_ERR_OK) && (list != NULL)) {
if (((ent->etype == XML_INTERNAL_GENERAL_ENTITY) ||
@@ -6372,6 +6476,8 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
} else if (list != NULL) {
xmlFreeNodeList(list);
list = NULL;
+ } else if (ent->owner != 1) {
+ ctxt->nbentities += ent->owner;
}
}
ent->checked = 1;
@@ -6428,7 +6534,6 @@ xmlParseReference(xmlParserCtxtPtr ctxt) {
}
return;
}
- ctxt->nbentities += ent->owner;
if ((ctxt->sax != NULL) && (ctxt->sax->reference != NULL) &&
(ctxt->replaceEntities == 0) && (!ctxt->disableSAX)) {
/*
@@ -6622,6 +6727,11 @@ xmlParseEntityRef(xmlParserCtxtPtr ctxt) {
if (RAW == ';') {
NEXT;
/*
+ * Increase the number of entity references parsed
+ */
+ ctxt->nbentities++;
+
+ /*
* Ask first SAX for entity resolution, otherwise try the
* predefined set.
*/
@@ -6793,6 +6903,10 @@ xmlParseStringEntityRef(xmlParserCtxtPtr ctxt, const xmlChar ** str) {
if (*ptr == ';') {
ptr++;
/*
+ * Increase the number of entity references parsed
+ */
+ ctxt->nbentities++;
+ /*
* Ask first SAX for entity resolution, otherwise try the
* predefined set.
*/
@@ -6954,6 +7068,11 @@ xmlParsePEReference(xmlParserCtxtPtr ctxt)
} else {
if (RAW == ';') {
NEXT;
+ /*
+ * Increase the number of entity references parsed
+ */
+ ctxt->nbentities++;
+
if ((ctxt->sax != NULL) &&
(ctxt->sax->getParameterEntity != NULL))
entity = ctxt->sax->getParameterEntity(ctxt->userData,
@@ -7164,6 +7283,11 @@ xmlParseStringPEReference(xmlParserCtxtPtr ctxt, const xmlChar **str) {
if (cur == ';') {
ptr++;
cur = *ptr;
+ /*
+ * Increase the number of entity references parsed
+ */
+ ctxt->nbentities++;
+
if ((ctxt->sax != NULL) &&
(ctxt->sax->getParameterEntity != NULL))
entity = ctxt->sax->getParameterEntity(ctxt->userData,
@@ -11517,7 +11641,7 @@ xmlParseCtxtExternalEntity(xmlParserCtxtPtr ctx, const xmlChar *URL,
if (ctx == NULL) return(-1);
- if ((ctx->depth > 40) || (ctx->nbentities >= 500000)) {
+ if (ctx->depth > 40) {
return(XML_ERR_ENTITY_LOOP);
}
@@ -11718,8 +11842,7 @@ xmlParseExternalEntityPrivate(xmlDocPtr doc, xmlParserCtxtPtr oldctxt,
xmlChar start[4];
xmlCharEncoding enc;
- if ((depth > 40) ||
- ((oldctxt != NULL) && (oldctxt->nbentities >= 500000))) {
+ if (depth > 40) {
return(XML_ERR_ENTITY_LOOP);
}
@@ -11857,6 +11980,25 @@ xmlParseExternalEntityPrivate(xmlDocPtr doc, xmlParserCtxtPtr oldctxt,
}
ret = XML_ERR_OK;
}
+
+ /*
+ * Record in the parent context the number of entities replacement
+ * done when parsing that reference.
+ */
+ oldctxt->nbentities += ctxt->nbentities;
+ /*
+ * Also record the size of the entity parsed
+ */
+ if (ctxt->input != NULL) {
+ oldctxt->sizeentities += ctxt->input->consumed;
+ oldctxt->sizeentities += (ctxt->input->cur - ctxt->input->base);
+ }
+ /*
+ * And record the last error if any
+ */
+ if (ctxt->lastError.code != XML_ERR_OK)
+ xmlCopyError(&ctxt->lastError, &oldctxt->lastError);
+
if (sax != NULL)
ctxt->sax = oldsax;
oldctxt->node_seq.maximum = ctxt->node_seq.maximum;
@@ -11963,7 +12105,7 @@ xmlParseBalancedChunkMemoryInternal(xmlParserCtxtPtr oldctxt,
int size;
xmlParserErrors ret = XML_ERR_OK;
- if ((oldctxt->depth > 40) || (oldctxt->nbentities >= 500000)) {
+ if (oldctxt->depth > 40) {
return(XML_ERR_ENTITY_LOOP);
}
@@ -12087,7 +12229,17 @@ xmlParseBalancedChunkMemoryInternal(xmlParserCtxtPtr oldctxt,
ctxt->myDoc->last = last;
}
+ /*
+ * Record in the parent context the number of entities replacement
+ * done when parsing that reference.
+ */
oldctxt->nbentities += ctxt->nbentities;
+ /*
+ * Also record the last error if any
+ */
+ if (ctxt->lastError.code != XML_ERR_OK)
+ xmlCopyError(&ctxt->lastError, &oldctxt->lastError);
+
ctxt->sax = oldsax;
ctxt->dict = NULL;
ctxt->attsDefault = NULL;
@@ -13404,6 +13556,7 @@ xmlCtxtReset(xmlParserCtxtPtr ctxt)
ctxt->charset = XML_CHAR_ENCODING_UTF8;
ctxt->catalogs = NULL;
ctxt->nbentities = 0;
+ ctxt->sizeentities = 0;
xmlInitNodeInfoSeq(&ctxt->node_seq);
if (ctxt->attsDefault != NULL) {
More information about the debian-xml-sgml-pkgs
mailing list