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;
