Make xmlParseConditionalSections non-recursive

Avoid call stack overflow in deeply nested conditional sections.

Found by OSS-Fuzz.

parser.c

@@ -6638,149 +6638,143 @@ xmlParseElementDecl(xmlParserCtxtPtr ctxt) {
 
 static void
 xmlParseConditionalSections(xmlParserCtxtPtr ctxt) {
+    int *inputIds = NULL;
+    size_t inputIdsSize = 0;
+    size_t depth = 0;
+
+    while (ctxt->instate != XML_PARSER_EOF) {
+        if ((RAW == '<') && (NXT(1) == '!') && (NXT(2) == '[')) {
             int id = ctxt->input->id;
 
             SKIP(3);
             SKIP_BLANKS;
+
             if (CMP7(CUR_PTR, 'I', 'N', 'C', 'L', 'U', 'D', 'E')) {
                 SKIP(7);
                 SKIP_BLANKS;
                 if (RAW != '[') {
                     xmlFatalErr(ctxt, XML_ERR_CONDSEC_INVALID, NULL);
                     xmlHaltParser(ctxt);
-	    return;
+                    goto error;
-	} else {
+                }
                 if (ctxt->input->id != id) {
                     xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
-	                       "All markup of the conditional section is not"
+                                   "All markup of the conditional section is"
-                               " in the same entity\n");
+                                   " not in the same entity\n");
                 }
                 NEXT;
-	}
-	if (xmlParserDebugEntities) {
-	    if ((ctxt->input != NULL) && (ctxt->input->filename))
-		xmlGenericError(xmlGenericErrorContext,
-			"%s(%d): ", ctxt->input->filename,
-			ctxt->input->line);
-	    xmlGenericError(xmlGenericErrorContext,
-		    "Entering INCLUDE Conditional Section\n");
-	}
 
-        SKIP_BLANKS;
+                if (inputIdsSize <= depth) {
-        GROW;
+                    int *tmp;
-	while (((RAW != 0) && ((RAW != ']') || (NXT(1) != ']') ||
-	        (NXT(2) != '>'))) && (ctxt->instate != XML_PARSER_EOF)) {
-	    const xmlChar *check = CUR_PTR;
-	    unsigned int cons = ctxt->input->consumed;
 
-	    if ((RAW == '<') && (NXT(1) == '!') && (NXT(2) == '[')) {
+                    inputIdsSize = (inputIdsSize == 0 ? 4 : inputIdsSize * 2);
-		xmlParseConditionalSections(ctxt);
+                    tmp = (int *) xmlRealloc(inputIds,
-	    } else
+                            inputIdsSize * sizeof(int));
-		xmlParseMarkupDecl(ctxt);
+                    if (tmp == NULL) {
-
+                        xmlErrMemory(ctxt, NULL);
-            SKIP_BLANKS;
+                        goto error;
-            GROW;
-
-	    if ((CUR_PTR == check) && (cons == ctxt->input->consumed)) {
-		xmlFatalErr(ctxt, XML_ERR_EXT_SUBSET_NOT_FINISHED, NULL);
-		xmlHaltParser(ctxt);
-		break;
                     }
+                    inputIds = tmp;
                 }
-	if (xmlParserDebugEntities) {
+                inputIds[depth] = id;
-	    if ((ctxt->input != NULL) && (ctxt->input->filename))
+                depth++;
-		xmlGenericError(xmlGenericErrorContext,
-			"%s(%d): ", ctxt->input->filename,
-			ctxt->input->line);
-	    xmlGenericError(xmlGenericErrorContext,
-		    "Leaving INCLUDE Conditional Section\n");
-	}
-
             } else if (CMP6(CUR_PTR, 'I', 'G', 'N', 'O', 'R', 'E')) {
                 int state;
                 xmlParserInputState instate;
-	int depth = 0;
+                size_t ignoreDepth = 0;
 
                 SKIP(6);
                 SKIP_BLANKS;
                 if (RAW != '[') {
                     xmlFatalErr(ctxt, XML_ERR_CONDSEC_INVALID, NULL);
                     xmlHaltParser(ctxt);
-	    return;
+                    goto error;
-	} else {
+                }
                 if (ctxt->input->id != id) {
                     xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
-	                       "All markup of the conditional section is not"
+                                   "All markup of the conditional section is"
-                               " in the same entity\n");
+                                   " not in the same entity\n");
                 }
                 NEXT;
-	}
-	if (xmlParserDebugEntities) {
-	    if ((ctxt->input != NULL) && (ctxt->input->filename))
-		xmlGenericError(xmlGenericErrorContext,
-			"%s(%d): ", ctxt->input->filename,
-			ctxt->input->line);
-	    xmlGenericError(xmlGenericErrorContext,
-		    "Entering IGNORE Conditional Section\n");
-	}
 
                 /*
-	 * Parse up to the end of the conditional section
+                 * Parse up to the end of the conditional section but disable
-	 * But disable SAX event generating DTD building in the meantime
+                 * SAX event generating DTD building in the meantime
                  */
                 state = ctxt->disableSAX;
                 instate = ctxt->instate;
                 if (ctxt->recovery == 0) ctxt->disableSAX = 1;
                 ctxt->instate = XML_PARSER_IGNORE;
 
-	while (((depth >= 0) && (RAW != 0)) &&
+                while (RAW != 0) {
-               (ctxt->instate != XML_PARSER_EOF)) {
                     if ((RAW == '<') && (NXT(1) == '!') && (NXT(2) == '[')) {
-	    depth++;
                         SKIP(3);
-	    continue;
+                        ignoreDepth++;
-	  }
+                        /* Check for integer overflow */
-	  if ((RAW == ']') && (NXT(1) == ']') && (NXT(2) == '>')) {
+                        if (ignoreDepth == 0) {
-	    if (--depth >= 0) SKIP(3);
+                            xmlErrMemory(ctxt, NULL);
-	    continue;
+                            goto error;
                         }
+                    } else if ((RAW == ']') && (NXT(1) == ']') &&
+                               (NXT(2) == '>')) {
+                        if (ignoreDepth == 0)
+                            break;
+                        SKIP(3);
+                        ignoreDepth--;
+                    } else {
                         NEXT;
-	  continue;
+                    }
                 }
 
                 ctxt->disableSAX = state;
                 ctxt->instate = instate;
 
-	if (xmlParserDebugEntities) {
+                if (ctxt->input->id != id) {
-	    if ((ctxt->input != NULL) && (ctxt->input->filename))
+                    xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
-		xmlGenericError(xmlGenericErrorContext,
+                                   "All markup of the conditional section is"
-			"%s(%d): ", ctxt->input->filename,
+                                   " not in the same entity\n");
-			ctxt->input->line);
-	    xmlGenericError(xmlGenericErrorContext,
-		    "Leaving IGNORE Conditional Section\n");
                 }
-
+                SKIP(3);
             } else {
                 xmlFatalErr(ctxt, XML_ERR_CONDSEC_INVALID_KEYWORD, NULL);
                 xmlHaltParser(ctxt);
-	return;
+                goto error;
+            }
+        } else if ((depth > 0) &&
+                   (RAW == ']') && (NXT(1) == ']') && (NXT(2) == '>')) {
+            depth--;
+            if (ctxt->input->id != inputIds[depth]) {
+                xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
+                               "All markup of the conditional section is not"
+                               " in the same entity\n");
+            }
+            SKIP(3);
+        } else {
+            const xmlChar *check = CUR_PTR;
+            unsigned int cons = ctxt->input->consumed;
+
+            xmlParseMarkupDecl(ctxt);
+
+            if ((CUR_PTR == check) && (cons == ctxt->input->consumed)) {
+                xmlFatalErr(ctxt, XML_ERR_EXT_SUBSET_NOT_FINISHED, NULL);
+                xmlHaltParser(ctxt);
+                goto error;
+            }
         }
 
-    if (RAW == 0)
+        if (depth == 0)
-        SHRINK;
+            break;
+
+        SKIP_BLANKS;
+        GROW;
+    }
 
     if (RAW == 0) {
 	xmlFatalErr(ctxt, XML_ERR_CONDSEC_NOT_FINISHED, NULL);
-    } else {
-	if (ctxt->input->id != id) {
-	    xmlFatalErrMsg(ctxt, XML_ERR_ENTITY_BOUNDARY,
-	                   "All markup of the conditional section is not in"
-                           " the same entity\n");
-	}
-	if ((ctxt-> instate != XML_PARSER_EOF) &&
-	    ((ctxt->input->cur + 3) <= ctxt->input->end))
-	    SKIP(3);
     }
+
+error:
+    xmlFree(inputIds);
 }
 
 /**
@@ -6842,16 +6836,6 @@ xmlParseMarkupDecl(xmlParserCtxtPtr ctxt) {
     if (ctxt->instate == XML_PARSER_EOF)
         return;
 
-    /*
-     * Conditional sections are allowed from entities included
-     * by PE References in the internal subset.
-     */
-    if ((ctxt->external == 0) && (ctxt->inputNr > 1)) {
-        if ((RAW == '<') && (NXT(1) == '!') && (NXT(2) == '[')) {
-	    xmlParseConditionalSections(ctxt);
-	}
-    }
-
     ctxt->instate = XML_PARSER_DTD;
 }
 
@@ -8315,6 +8299,15 @@ xmlParseInternalSubset(xmlParserCtxtPtr ctxt) {
 	    xmlParseMarkupDecl(ctxt);
 	    xmlParsePEReference(ctxt);
 
+            /*
+             * Conditional sections are allowed from entities included
+             * by PE References in the internal subset.
+             */
+            if ((ctxt->inputNr > 1) &&
+                (RAW == '<') && (NXT(1) == '!') && (NXT(2) == '[')) {
+                xmlParseConditionalSections(ctxt);
+            }
+
 	    if ((CUR_PTR == check) && (cons == ctxt->input->consumed)) {
 		xmlFatalErr(ctxt, XML_ERR_INTERNAL_ERROR,
 	     "xmlParseInternalSubset: error detected in Markup declaration\n");

result/errors/759573-2.xml.err

@@ -46,13 +46,3 @@ Entity: line 3:
 Entity: line 3: 
 %zz;<!ELEMENTD(%MENT%MENTDŹMENTD%zNMT9KENSMYSYSTEM;MENT9%zz;
              ^
-./test/errors/759573-2.xml:6: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
-
-
-^
-./test/errors/759573-2.xml:6: parser error : DOCTYPE improperly terminated
-
-^
-./test/errors/759573-2.xml:6: parser error : Start tag expected, '<' not found
-
-^

result/errors/759573.xml.err

@@ -19,13 +19,3 @@ T t (A)><!ENTITY % xx '&#37;<![INCLUDE[000&#37;&#3000;000&#37;z;'><!ENTITYz>%xx;
 Entity: line 1: 
 %<![INCLUDE[000%ஸ000%z;
             ^
-./test/errors/759573.xml:1: parser error : internal error: xmlParseInternalSubset: error detected in Markup declaration
-
-
-^
-./test/errors/759573.xml:1: parser error : DOCTYPE improperly terminated
-
-^
-./test/errors/759573.xml:1: parser error : Start tag expected, '<' not found
-
-^

result/valid/cond_sect1.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<!DOCTYPE doc SYSTEM "dtds/cond_sect1.dtd" [
+<!ENTITY % include "INCLUDE">
+<!ENTITY % ignore "IGNORE">
+]>
+<doc>
+    <child>text</child>
+</doc>

result/valid/cond_sect2.xml.err

@@ -0,0 +1,9 @@
+test/valid/dtds/cond_sect2.dtd:15: parser error : All markup of the conditional section is not in the same entity
+    %ent;
+         ^
+Entity: line 1: 
+]]>
+^
+test/valid/dtds/cond_sect2.dtd:17: parser error : Content error in the external subset
+
+^

result/valid/cond_sect2.xml.err.rdr

@@ -0,0 +1,10 @@
+test/valid/dtds/cond_sect2.dtd:15: parser error : All markup of the conditional section is not in the same entity
+    %ent;
+         ^
+Entity: line 1: 
+]]>
+^
+test/valid/dtds/cond_sect2.dtd:17: parser error : Content error in the external subset
+
+^
+./test/valid/cond_sect2.xml : failed to parse

test/valid/cond_sect1.xml

@@ -0,0 +1,7 @@
+<!DOCTYPE doc SYSTEM "dtds/cond_sect1.dtd" [
+    <!ENTITY % include "INCLUDE">
+    <!ENTITY % ignore  "IGNORE">
+]>
+<doc>
+    <child>text</child>
+</doc>

test/valid/cond_sect2.xml

@@ -0,0 +1,4 @@
+<!DOCTYPE doc SYSTEM "dtds/cond_sect2.dtd">
+<doc>
+    <child>text</child>
+</doc>

test/valid/dtds/cond_sect1.dtd

@@ -0,0 +1,20 @@
+<![ %include; [
+    <![%include; [
+        <![ %include;[
+            <![%include;[
+                <!ELEMENT doc (child)>
+                <!ELEMENT child (#PCDATA)>
+            ]]>
+        ]]>
+    ]]>
+]]>
+<![ %ignore; [
+    <![%include; [
+        <![ %include;[
+            <![%ignore;[
+                <!ELEMENT doc (x)>
+                <!ELEMENT child (y)>
+            ]]>
+        ]]>
+    ]]>
+]]>

test/valid/dtds/cond_sect2.dtd

@@ -0,0 +1,16 @@
+<!ENTITY % ent "]]>">
+<![INCLUDE[
+    <![INCLUDE[
+        <![INCLUDE[
+            <![INCLUDE[
+                <![INCLUDE[
+                    <![INCLUDE[
+                        <![INCLUDE[
+                            <![INCLUDE[
+                        ]]>
+                    ]]>
+                ]]>
+            ]]>
+        ]]>
+    %ent;
+]]>