CVE-2019-15903: Improper Restriction of Recursive Entity References in DTDs ('XML Entity Expansion') (CWE-776)
value: In libexpat before 2.2.8, crafted XML input could fool the parser into changing from DTD parsing to document parsing too early; a consecutive call to XML_GetCurrentLineNumber (or XML_GetCurrentColumnNumber) then resulted in a heap-based buffer over-read.
CWE description: The product uses XML documents and allows their structure to be defined with a Document Type Definition (DTD), but it does not properly control the number of recursive definitions of entities.
CVSS v3 base score: 7.5
Published: 2019-09-04T06:15Z
Repository: https://github.com/libexpat/libexpat
Language: C
Files changed: expat/lib/xmlparse.c
Commit date: 2019-08-28 15:14:26 +0200
Commit message: xmlparse.c: Deny internal entities closing the doctype

Diff: diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c
index 0553e3d..c29a644 100644
--- a/expat/lib/xmlparse.c
+++ b/expat/lib/xmlparse.c
@@ -399,11 +399,11 @@ static enum XML_Error processXmlDecl(XML_Parser parser, int isGeneralTextEntity,
                                      const char *s, const char *next);
 static enum XML_Error initializeEncoding(XML_Parser parser);
 static enum XML_Error doProlog(XML_Parser parser, const ENCODING *enc,
                                const char *s, const char *end, int tok,
                                const char *next, const char **nextPtr,
-                               XML_Bool haveMore);
+                               XML_Bool haveMore, XML_Bool allowClosingDoctype);
 static enum XML_Error processInternalEntity(XML_Parser parser, ENTITY *entity,
                                             XML_Bool betweenDecl);
 static enum XML_Error doContent(XML_Parser parser, int startTagLevel,
                                 const ENCODING *enc, const char *start,
                                 const char *end, const char **endPtr,
@@ -4044,11 +4044,11 @@ externalParEntProcessor(XML_Parser parser, const char *s, const char *end,
     tok = XmlPrologTok(parser->m_encoding, s, end, &next);
   }
 
   parser->m_processor = prologProcessor;
   return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr,
-                  (XML_Bool)! parser->m_parsingStatus.finalBuffer);
+                  (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
 }
 
 static enum XML_Error PTRCALL
 entityValueProcessor(XML_Parser parser, const char *s, const char *end,
                      const char **nextPtr) {
@@ -4088,16 +4088,17 @@ static enum XML_Error PTRCALL
 prologProcessor(XML_Parser parser, const char *s, const char *end,
                 const char **nextPtr) {
   const char *next = s;
   int tok = XmlPrologTok(parser->m_encoding, s, end, &next);
   return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr,
-                  (XML_Bool)! parser->m_parsingStatus.finalBuffer);
+                  (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
 }
 
 static enum XML_Error
 doProlog(XML_Parser parser, const ENCODING *enc, const char *s, const char *end,
-         int tok, const char *next, const char **nextPtr, XML_Bool haveMore) {
+         int tok, const char *next, const char **nextPtr, XML_Bool haveMore,
+         XML_Bool allowClosingDoctype) {
 #ifdef XML_DTD
   static const XML_Char externalSubsetName[] = {ASCII_HASH, '\0'};
 #endif /* XML_DTD */
   static const XML_Char atypeCDATA[]
       = {ASCII_C, ASCII_D, ASCII_A, ASCII_T, ASCII_A, '\0'};
@@ -4269,10 +4270,15 @@ doProlog(XML_Parser parser, const ENCODING *enc, const char *s, const char *end,
         if (parser->m_entityDeclHandler && role == XML_ROLE_ENTITY_PUBLIC_ID)
           handleDefault = XML_FALSE;
       }
       break;
     case XML_ROLE_DOCTYPE_CLOSE:
+      if (allowClosingDoctype != XML_TRUE) {
+        /* Must not close doctype from within expanded parameter entities */
+        return XML_ERROR_INVALID_TOKEN;
+      }
+
       if (parser->m_doctypeName) {
         parser->m_startDoctypeDeclHandler(
             parser->m_handlerArg, parser->m_doctypeName, parser->m_doctypeSysid,
             parser->m_doctypePubid, 0);
         poolClear(&parser->m_tempPool);
@@ -5172,11 +5178,11 @@ processInternalEntity(XML_Parser parser, ENTITY *entity, XML_Bool betweenDecl) {
 #ifdef XML_DTD
   if (entity->is_param) {
     int tok
         = XmlPrologTok(parser->m_internalEncoding, textStart, textEnd, &next);
     result = doProlog(parser, parser->m_internalEncoding, textStart, textEnd,
-                      tok, next, &next, XML_FALSE);
+                      tok, next, &next, XML_FALSE, XML_FALSE);
   } else
 #endif /* XML_DTD */
     result = doContent(parser, parser->m_tagLevel, parser->m_internalEncoding,
                        textStart, textEnd, &next, XML_FALSE);
 
@@ -5215,11 +5221,11 @@ internalEntityProcessor(XML_Parser parser, const char *s, const char *end,
 #ifdef XML_DTD
   if (entity->is_param) {
     int tok
         = XmlPrologTok(parser->m_internalEncoding, textStart, textEnd, &next);
     result = doProlog(parser, parser->m_internalEncoding, textStart, textEnd,
-                      tok, next, &next, XML_FALSE);
+                      tok, next, &next, XML_FALSE, XML_TRUE);
   } else
 #endif /* XML_DTD */
     result = doContent(parser, openEntity->startTagLevel,
                        parser->m_internalEncoding, textStart, textEnd, &next,
                        XML_FALSE);
@@ -5242,11 +5248,11 @@ internalEntityProcessor(XML_Parser parser, const char *s, const char *end,
   if (entity->is_param) {
     int tok;
     parser->m_processor = prologProcessor;
     tok = XmlPrologTok(parser->m_encoding, s, end, &next);
     return doProlog(parser, parser->m_encoding, s, end, tok, next, nextPtr,
-                    (XML_Bool)! parser->m_parsingStatus.finalBuffer);
+                    (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
   } else
 #endif /* XML_DTD */
   {
     parser->m_processor = contentProcessor;
     /* see externalEntityContentProcessor vs contentProcessor */

Vulnerable code:                                XML_Bool haveMore);
                  (XML_Bool)! parser->m_parsingStatus.finalBuffer);
                  (XML_Bool)! parser->m_parsingStatus.finalBuffer);
         int tok, const char *next, const char **nextPtr, XML_Bool haveMore) {
                      tok, next, &next, XML_FALSE);
                      tok, next, &next, XML_FALSE);
                    (XML_Bool)! parser->m_parsingStatus.finalBuffer);
Fixed code:                                XML_Bool haveMore, XML_Bool allowClosingDoctype);
                  (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
                  (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);
         int tok, const char *next, const char **nextPtr, XML_Bool haveMore,
         XML_Bool allowClosingDoctype) {
      if (allowClosingDoctype != XML_TRUE) {
        /* Must not close doctype from within expanded parameter entities */
        return XML_ERROR_INVALID_TOKEN;
      }

                      tok, next, &next, XML_FALSE, XML_FALSE);
                      tok, next, &next, XML_FALSE, XML_TRUE);
                    (XML_Bool)! parser->m_parsingStatus.finalBuffer, XML_TRUE);