Make the XML entity recursion check more precise.
libxml doesn't detect entity recursion specifically but has a variety
of related checks, such as entities not expanding too deeply or
producing exponential blow-ups in content.
Because entity declarations are parsed in a separate context with
their own element recursion budget, a recursive entity can overflow
the stack using a lot of open elements (but within the per-context
limit) as it slowly consumes (but does not exhaust) the entity depth
budget.
This adds a specific, precise check for recursive entities that
detects entity recursion specifically and fails immediately.
The existing entity expansion depth checks are still relevant for long
chains of different entities.
BUG=628581
Review-Url: https://codereview.chromium.org/2539003002
Cr-Commit-Position: refs/heads/master@{#436899}
Index: libxml2-2.9.4/entities.c
===================================================================
--- libxml2-2.9.4.orig/entities.c
+++ libxml2-2.9.4/entities.c
@@ -159,6 +159,7 @@ xmlCreateEntity(xmlDictPtr dict, const x
memset(ret, 0, sizeof(xmlEntity));
ret->type = XML_ENTITY_DECL;
ret->checked = 0;
+ ret->guard = XML_ENTITY_NOT_BEING_CHECKED;
/*
* fill the structure.
@@ -931,6 +932,7 @@ xmlCopyEntity(xmlEntityPtr ent) {
cur->orig = xmlStrdup(ent->orig);
if (ent->URI != NULL)
cur->URI = xmlStrdup(ent->URI);
+ cur->guard = 0;
return(cur);
}
Index: libxml2-2.9.4/include/libxml/entities.h
===================================================================
--- libxml2-2.9.4.orig/include/libxml/entities.h
+++ libxml2-2.9.4/include/libxml/entities.h
@@ -30,6 +30,11 @@ typedef enum {
XML_INTERNAL_PREDEFINED_ENTITY = 6
} xmlEntityType;
+typedef enum {
+ XML_ENTITY_NOT_BEING_CHECKED,
+ XML_ENTITY_BEING_CHECKED /* entity check is in progress */
+} xmlEntityRecursionGuard;
+
/*
* An unit of storage for an entity, contains the string, the value
* and the linkind data needed for the linking in the hash table.
@@ -60,6 +65,7 @@ struct _xmlEntity {
/* this is also used to count entities
* references done from that entity
* and if it contains '<' */
+ xmlEntityRecursionGuard guard;
};
/*
Index: libxml2-2.9.4/parser.c
===================================================================
--- libxml2-2.9.4.orig/parser.c
+++ libxml2-2.9.4/parser.c
@@ -133,6 +133,10 @@ xmlParserEntityCheck(xmlParserCtxtPtr ct
if (ctxt->lastError.code == XML_ERR_ENTITY_LOOP)
return (1);
+ if ((ent != NULL) && (ent->guard == XML_ENTITY_BEING_CHECKED)) {
+ xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
+ return (1);
+ }
/*
* This may look absurd but is needed to detect
* entities problems
@@ -143,12 +147,14 @@ xmlParserEntityCheck(xmlParserCtxtPtr ct
unsigned long oldnbent = ctxt->nbentities;
xmlChar *rep;
+ ent->guard = XML_ENTITY_BEING_CHECKED;
ent->checked = 1;
++ctxt->depth;
rep = xmlStringDecodeEntities(ctxt, ent->content,
XML_SUBSTITUTE_REF, 0, 0, 0);
--ctxt->depth;
+ ent->guard = XML_ENTITY_NOT_BEING_CHECKED;
if (ctxt->errNo == XML_ERR_ENTITY_LOOP) {
ent->content[0] = 0;
}
@@ -7337,23 +7343,28 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
* if its replacement text matches the production labeled
* content.
*/
- if (ent->etype == XML_INTERNAL_GENERAL_ENTITY) {
- ctxt->depth++;
- ret = xmlParseBalancedChunkMemoryInternal(ctxt, ent->content,
- user_data, &list);
- ctxt->depth--;
-
- } else if (ent->etype == XML_EXTERNAL_GENERAL_PARSED_ENTITY) {
- ctxt->depth++;
- ret = xmlParseExternalEntityPrivate(ctxt->myDoc, ctxt, ctxt->sax,
- user_data, ctxt->depth, ent->URI,
- ent->ExternalID, &list);
- ctxt->depth--;
- } else {
- ret = XML_ERR_ENTITY_PE_INTERNAL;
- xmlErrMsgStr(ctxt, XML_ERR_INTERNAL_ERROR,
- "invalid entity type found\n", NULL);
- }
+ if (ent->guard == XML_ENTITY_BEING_CHECKED) {
+ ret = XML_ERR_ENTITY_LOOP;
+ } else {
+ ent->guard = XML_ENTITY_BEING_CHECKED;
+ if (ent->etype == XML_INTERNAL_GENERAL_ENTITY) {
+ ctxt->depth++;
+ ret = xmlParseBalancedChunkMemoryInternal(ctxt, ent->content,
+ user_data, &list);
+ ctxt->depth--;
+ } else if (ent->etype == XML_EXTERNAL_GENERAL_PARSED_ENTITY) {
+ ctxt->depth++;
+ ret = xmlParseExternalEntityPrivate(ctxt->myDoc, ctxt, ctxt->sax,
+ user_data, ctxt->depth, ent->URI,
+ ent->ExternalID, &list);
+ ctxt->depth--;
+ } else {
+ ret = XML_ERR_ENTITY_PE_INTERNAL;
+ xmlErrMsgStr(ctxt, XML_ERR_INTERNAL_ERROR,
+ "invalid entity type found\n", NULL);
+ }
+ ent->guard = XML_ENTITY_NOT_BEING_CHECKED;
+ }
/*
* Store the number of entities needing parsing for this entity
@@ -7456,23 +7467,29 @@ xmlParseReference(xmlParserCtxtPtr ctxt)
else
user_data = ctxt->userData;
- if (ent->etype == XML_INTERNAL_GENERAL_ENTITY) {
- ctxt->depth++;
- ret = xmlParseBalancedChunkMemoryInternal(ctxt,
- ent->content, user_data, NULL);
- ctxt->depth--;
- } else if (ent->etype ==
- XML_EXTERNAL_GENERAL_PARSED_ENTITY) {
- ctxt->depth++;
- ret = xmlParseExternalEntityPrivate(ctxt->myDoc, ctxt,
- ctxt->sax, user_data, ctxt->depth,
- ent->URI, ent->ExternalID, NULL);
- ctxt->depth--;
- } else {
- ret = XML_ERR_ENTITY_PE_INTERNAL;
- xmlErrMsgStr(ctxt, XML_ERR_INTERNAL_ERROR,
- "invalid entity type found\n", NULL);
- }
+ if (ent->guard == XML_ENTITY_BEING_CHECKED) {
+ ret = XML_ERR_ENTITY_LOOP;
+ } else {
+ ent->guard = XML_ENTITY_BEING_CHECKED;
+ if (ent->etype == XML_INTERNAL_GENERAL_ENTITY) {
+ ctxt->depth++;
+ ret = xmlParseBalancedChunkMemoryInternal(ctxt,
+ ent->content, user_data, NULL);
+ ctxt->depth--;
+ } else if (ent->etype ==
+ XML_EXTERNAL_GENERAL_PARSED_ENTITY) {
+ ctxt->depth++;
+ ret = xmlParseExternalEntityPrivate(ctxt->myDoc, ctxt,
+ ctxt->sax, user_data, ctxt->depth,
+ ent->URI, ent->ExternalID, NULL);
+ ctxt->depth--;
+ } else {
+ ret = XML_ERR_ENTITY_PE_INTERNAL;
+ xmlErrMsgStr(ctxt, XML_ERR_INTERNAL_ERROR,
+ "invalid entity type found\n", NULL);
+ }
+ ent->guard = XML_ENTITY_NOT_BEING_CHECKED;
+ }
if (ret == XML_ERR_ENTITY_LOOP) {
xmlFatalErr(ctxt, XML_ERR_ENTITY_LOOP, NULL);
return;