expat/backport-005-CVE-2024-8176.patch

From da10ca23282cf336553307dcc47dafb5b7378ad0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Berkay=20Eren=20=C3=9Cr=C3=BCn?= <berkay.ueruen@siemens.com>
Date: Tue, 27 Aug 2024 17:14:50 +0200
Subject: [PATCH 5/7] Break cyclic appendAttributeValue recursion

During processing attributes with entity references,
appendAttributeValue can reach high recursion depths that can lead to
a crash.

This commit switches the processing to an iterative approach similar to
the fix for internal entity processing. A new m_openAttributeEntities
list is introduced to keep track of entity references that need
processing. When a new entity reference is detected, instead of calling
appendAttributeValue recursively, the entity will be added to open
entities list and the execution will return to storeAttributeValue,
where newly added entity will be handled. After the entity processing
is done, appendAttributeValue will be called by using the next token.

Reference: https://github.com/libexpat/libexpat/pull/973/commits/da10ca23282cf336553307dcc47dafb5b7378ad0
Conflict: adapt appendAttributeValue

---
 lib/xmlparse.c | 145 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 127 insertions(+), 18 deletions(-)

diff --git a/lib/xmlparse.c b/lib/xmlparse.c
index 781fc71..c86da91 100644
--- a/lib/xmlparse.c
+++ b/lib/xmlparse.c
@@ -457,6 +457,8 @@ static enum XML_Error doProlog(XML_Parser parser, const ENCODING *enc,
                                enum XML_Account account);
 static enum XML_Error processInternalEntity(XML_Parser parser, ENTITY *entity,
                                             XML_Bool betweenDecl);
+static enum XML_Error processAttributeEntity(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,
@@ -665,6 +667,8 @@ struct XML_ParserStruct {
   const char *m_positionPtr;
   OPEN_INTERNAL_ENTITY *m_openInternalEntities;
   OPEN_INTERNAL_ENTITY *m_freeInternalEntities;
+  OPEN_INTERNAL_ENTITY *m_openAttributeEntities;
+  OPEN_INTERNAL_ENTITY *m_freeAttributeEntities;
   XML_Bool m_defaultExpandInternalEntities;
   int m_tagLevel;
   ENTITY *m_declEntity;
@@ -1103,6 +1107,7 @@ parserCreate(const XML_Char *encodingName,
   parser->m_freeBindingList = NULL;
   parser->m_freeTagList = NULL;
   parser->m_freeInternalEntities = NULL;
+  parser->m_freeAttributeEntities = NULL;
 
   parser->m_groupSize = 0;
   parser->m_groupConnector = NULL;
@@ -1204,6 +1209,7 @@ parserInit(XML_Parser parser, const XML_Char *encodingName) {
   parser->m_eventEndPtr = NULL;
   parser->m_positionPtr = NULL;
   parser->m_openInternalEntities = NULL;
+  parser->m_openAttributeEntities = NULL;
   parser->m_defaultExpandInternalEntities = XML_TRUE;
   parser->m_tagLevel = 0;
   parser->m_tagStack = NULL;
@@ -1275,6 +1281,15 @@ XML_ParserReset(XML_Parser parser, const XML_Char *encodingName) {
     openEntity->next = parser->m_freeInternalEntities;
     parser->m_freeInternalEntities = openEntity;
   }
+  /* move m_openAttributeEntities to m_freeAttributeEntities (i.e. same task but
+   * for attributes) */
+  openEntityList = parser->m_openAttributeEntities;
+  while (openEntityList) {
+    OPEN_INTERNAL_ENTITY *openEntity = openEntityList;
+    openEntityList = openEntity->next;
+    openEntity->next = parser->m_freeAttributeEntities;
+    parser->m_freeAttributeEntities = openEntity;
+  }
   moveToFreeBindingList(parser, parser->m_inheritedBindings);
   FREE(parser, parser->m_unknownEncodingMem);
   if (parser->m_unknownEncodingRelease)
@@ -1531,7 +1546,20 @@ XML_ParserFree(XML_Parser parser) {
     entityList = entityList->next;
     FREE(parser, openEntity);
   }
-
+  /* free m_openAttributeEntities and m_freeAttributeEntities */
+  entityList = parser->m_openAttributeEntities;
+  for (;;) {
+    OPEN_INTERNAL_ENTITY *openEntity;
+    if (entityList == NULL) {
+      if (parser->m_freeAttributeEntities == NULL)
+        break;
+      entityList = parser->m_freeAttributeEntities;
+      parser->m_freeAttributeEntities = NULL;
+    }
+    openEntity = entityList;
+    entityList = entityList->next;
+    FREE(parser, openEntity);
+  }
   destroyBindings(parser->m_freeBindingList, parser);
   destroyBindings(parser->m_inheritedBindings, parser);
   poolDestroy(&parser->m_tempPool);
@@ -5859,6 +5887,7 @@ epilogProcessor(XML_Parser parser, const char *s, const char *end,
   }
 }
 
+/* KEEP IN SYNC with processAttributeEntity */
 static enum XML_Error
 processInternalEntity(XML_Parser parser, ENTITY *entity, XML_Bool betweenDecl) {
   OPEN_INTERNAL_ENTITY *openEntity;
@@ -5890,6 +5919,37 @@ processInternalEntity(XML_Parser parser, ENTITY *entity, XML_Bool betweenDecl) {
   return XML_ERROR_NONE;
 }
 
+/* KEEP IN SYNC with processInternalEntity */
+static enum XML_Error
+processAttributeEntity(XML_Parser parser, ENTITY *entity,
+                       XML_Bool betweenDecl) {
+  OPEN_INTERNAL_ENTITY *openEntity;
+
+  if (parser->m_freeAttributeEntities) {
+    openEntity = parser->m_freeAttributeEntities;
+    parser->m_freeAttributeEntities = openEntity->next;
+  } else {
+    openEntity
+        = (OPEN_INTERNAL_ENTITY *)MALLOC(parser, sizeof(OPEN_INTERNAL_ENTITY));
+    if (! openEntity)
+      return XML_ERROR_NO_MEMORY;
+  }
+  entity->open = XML_TRUE;
+#if XML_GE == 1
+  entityTrackingOnOpen(parser, entity, __LINE__);
+#endif
+  entity->processed = 0;
+  openEntity->next = parser->m_openAttributeEntities;
+  parser->m_openAttributeEntities = openEntity;
+  openEntity->entity = entity;
+  openEntity->startTagLevel = parser->m_tagLevel;
+  openEntity->betweenDecl = betweenDecl;
+  openEntity->internalEventPtr = NULL;
+  openEntity->internalEventEndPtr = NULL;
+
+  return XML_ERROR_NONE;
+}
+
 static enum XML_Error PTRCALL
 internalEntityProcessor(XML_Parser parser, const char *s, const char *end,
                         const char **nextPtr) {
@@ -5994,8 +6054,67 @@ static enum XML_Error
 storeAttributeValue(XML_Parser parser, const ENCODING *enc, XML_Bool isCdata,
                     const char *ptr, const char *end, STRING_POOL *pool,
                     enum XML_Account account) {
-  enum XML_Error result = appendAttributeValue(parser, enc, isCdata, ptr, end,
-                                               pool, account, NULL);
+  const char *next = ptr;
+  enum XML_Error result = XML_ERROR_NONE;
+
+  while (1) {
+    if (! parser->m_openAttributeEntities) {
+      result = appendAttributeValue(parser, enc, isCdata, next, end, pool,
+                                    account, &next);
+    } else {
+      OPEN_INTERNAL_ENTITY *const openEntity = parser->m_openAttributeEntities;
+      if (! openEntity)
+        return XML_ERROR_UNEXPECTED_STATE;
+
+      ENTITY *const entity = openEntity->entity;
+      const char *const textStart
+          = ((const char *)entity->textPtr) + entity->processed;
+      const char *const textEnd
+          = (const char *)(entity->textPtr + entity->textLen);
+      /* Set a safe default value in case 'next' does not get set */
+      const char *nextInEntity = textStart;
+      result = appendAttributeValue(
+          parser, parser->m_internalEncoding, isCdata, textStart, textEnd, pool,
+          XML_ACCOUNT_ENTITY_EXPANSION, &nextInEntity);
+      if (result != XML_ERROR_NONE)
+        break;
+      // Check if entity is complete, if not, mark down how much of it is
+      // processed. A XML_SUSPENDED check here is not required as
+      // appendAttributeValue will never suspend the parser.
+      if (textEnd != nextInEntity) {
+        entity->processed = (int)(nextInEntity - (const char *)entity->textPtr);
+        continue;
+      }
+
+#if XML_GE == 1
+      entityTrackingOnClose(parser, entity, __LINE__);
+#endif
+      entity->open = XML_FALSE;
+      // Remove fully processed openEntity from open entity list.
+      // appendAttributeValue call can add maximum one new entity to
+      // m_openAttributeEntities since when a new entity is detected, parser
+      // will set the 'reenter' flag and return. Therefore openEntity is either
+      // the head or next to the new head.
+      if (parser->m_openAttributeEntities == openEntity) {
+        // No new entity detected during entity processing,
+        // openEntity is still the head.
+        parser->m_openAttributeEntities = parser->m_openAttributeEntities->next;
+      } else {
+        // New entity detected since list has a new head. openEntity is the
+        // second element.
+        parser->m_openAttributeEntities->next = openEntity->next;
+      }
+      /* put openEntity back in list of free instances */
+      openEntity->next = parser->m_freeAttributeEntities;
+      parser->m_freeAttributeEntities = openEntity;
+    }
+
+    // Break if an error occurred or there is nothing left to process
+    if (result || (parser->m_openAttributeEntities == NULL && end == next)) {
+      break;
+    }
+  }
+
   if (result)
     return result;
   if (! isCdata && poolLength(pool) && poolLastChar(pool) == 0x20)
@@ -6169,21 +6288,11 @@ appendAttributeValue(XML_Parser parser, const ENCODING *enc, XML_Bool isCdata,
         return XML_ERROR_ATTRIBUTE_EXTERNAL_ENTITY_REF;
       } else {
         enum XML_Error result;
-        const XML_Char *textEnd = entity->textPtr + entity->textLen;
-        entity->open = XML_TRUE;
-#if defined(XML_DTD) || XML_GE == 1
-        entityTrackingOnOpen(parser, entity, __LINE__);
-#endif
-        result = appendAttributeValue(parser, parser->m_internalEncoding,
-                                      isCdata, (const char *)entity->textPtr,
-                                      (const char *)textEnd, pool,
-                                      XML_ACCOUNT_ENTITY_EXPANSION, NULL);
-#if defined(XML_DTD) || XML_GE == 1
-        entityTrackingOnClose(parser, entity, __LINE__);
-#endif
-        entity->open = XML_FALSE;
-        if (result)
-          return result;
+        result = processAttributeEntity(parser, entity, XML_FALSE);
+        if (nextPtr) {
+          *nextPtr = next;
+        }
+        return result;        
       }
     } break;
     default:
-- 
2.33.0