From: Daniel Veillard Date: Thu, 16 Oct 2014 13:59:47 +0800 Subject: Fix for CVE-2014-3660 Issues related to the billion laugh entity expansion which happened to escape the initial set of fixes --- parser.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/parser.c b/parser.c index ab69d56..b7f3c03 100644 --- a/parser.c +++ b/parser.c @@ -130,6 +130,29 @@ xmlParserEntityCheck(xmlParserCtxtPtr ctxt, size_t size, return (0); if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP) return (1); + + /* + * This may look absurd but is needed to detect + * entities problems + */ + if ((ent != NULL) && (ent->etype != XML_INTERNAL_PREDEFINED_ENTITY) && + (ent->content != NULL) && (ent->checked == 0)) { + unsigned long oldnbent = ctxt->nbentities; + xmlChar *rep; + + ent->checked = 1; + + rep = xmlStringDecodeEntities(ctxt, ent->content, + XML_SUBSTITUTE_REF, 0, 0, 0); + + ent->checked = (ctxt->nbentities - oldnbent + 1) * 2; + if (rep != NULL) { + if (xmlStrchr(rep, '<')) + ent->checked |= 1; + xmlFree(rep); + rep = NULL; + } + } if (replacement != 0) { if (replacement < XML_MAX_TEXT_LENGTH) return(0); @@ -189,9 +212,12 @@ xmlParserEntityCheck(xmlParserCtxtPtr ctxt, size_t size, return (0); } else { /* - * strange we got no data for checking just return + * strange we got no data for checking */ - return (0); + if (((ctxt->lastError.code != XML_ERR_UNDECLARED_ENTITY) && + (ctxt->lastError.code != XML_WAR_UNDECLARED_ENTITY)) || + (ctxt->nbentities <= 10000)) + return (0); } xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL); return (1); @@ -2584,6 +2610,7 @@ xmlParserHandlePEReference(xmlParserCtxtPtr ctxt) { name, NULL); ctxt->valid = 0; } + xmlParserEntityCheck(ctxt, 0, NULL, 0); } else if (ctxt->input->free != deallocblankswrapper) { input = xmlNewBlanksWrapperInputStream(ctxt, entity); if (xmlPushInput(ctxt, input) < 0) @@ -2754,6 +2781,7 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len, 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 / 2; if ((ent != NULL) && @@ -2805,6 +2833,7 @@ xmlStringLenDecodeEntities(xmlParserCtxtPtr ctxt, const xmlChar *str, int len, 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 / 2; if (ent != NULL) { @@ -7307,6 +7336,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt) { (ret != XML_WAR_UNDECLARED_ENTITY)) { xmlFatalErrMsgStr(ctxt, XML_ERR_UNDECLARED_ENTITY, "Entity '%s' failed to parse\n", ent->name); + xmlParserEntityCheck(ctxt, 0, ent, 0); } else if (list != NULL) { xmlFreeNodeList(list); list = NULL; @@ -7413,7 +7443,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt) { /* * We are copying here, make sure there is no abuse */ - ctxt->sizeentcopy += ent->length; + ctxt->sizeentcopy += ent->length + 5; if (xmlParserEntityCheck(ctxt, 0, ent, ctxt->sizeentcopy)) return; @@ -7461,7 +7491,7 @@ xmlParseReference(xmlParserCtxtPtr ctxt) { /* * We are copying here, make sure there is no abuse */ - ctxt->sizeentcopy += ent->length; + ctxt->sizeentcopy += ent->length + 5; if (xmlParserEntityCheck(ctxt, 0, ent, ctxt->sizeentcopy)) return; @@ -7647,6 +7677,7 @@ xmlParseEntityRef(xmlParserCtxtPtr ctxt) { ctxt->sax->reference(ctxt->userData, name); } } + xmlParserEntityCheck(ctxt, 0, ent, 0); ctxt->valid = 0; } @@ -7840,6 +7871,7 @@ xmlParseStringEntityRef(xmlParserCtxtPtr ctxt, const xmlChar ** str) { "Entity '%s' not defined\n", name); } + xmlParserEntityCheck(ctxt, 0, ent, 0); /* TODO ? check regressions ctxt->valid = 0; */ } @@ -7999,6 +8031,7 @@ xmlParsePEReference(xmlParserCtxtPtr ctxt) name, NULL); ctxt->valid = 0; } + xmlParserEntityCheck(ctxt, 0, NULL, 0); } else { /* * Internal checking in case the entity quest barfed @@ -8238,6 +8271,7 @@ xmlParseStringPEReference(xmlParserCtxtPtr ctxt, const xmlChar **str) { name, NULL); ctxt->valid = 0; } + xmlParserEntityCheck(ctxt, 0, NULL, 0); } else { /* * Internal checking in case the entity quest barfed