[CVE-2022-40304] Fix dict corruption caused by entity reference cycles 87/287287/1 accepted/tizen_6.5_base accepted/tizen_6.5_base_tool tizen_6.5_base accepted/tizen/6.5/base/20230714.002546 accepted/tizen/6.5/base/tool/20230131.025508 submit/tizen_6.5_base/20230126.073210
authorNick Wellnhofer <wellnhofer@aevum.de>
Wed, 31 Aug 2022 20:11:25 +0000 (22:11 +0200)
committerDongHun Kwak <dh0128.kwak@samsung.com>
Thu, 26 Jan 2023 06:59:13 +0000 (15:59 +0900)
When an entity reference cycle is detected, the entity content is
cleared by setting its first byte to zero. But the entity content might
be allocated from a dict. In this case, the dict entry becomes corrupted
leading to all kinds of logic errors, including memory errors like
double-frees.

Stop storing entity content, orig, ExternalID and SystemID in a dict.
These values are unlikely to occur multiple times in a document, so they
shouldn't have been stored in a dict in the first place.

Thanks to Ned Williamson and Nathan Wachholz working with Google Project
Zero for the report!

Change-Id: Ic72980e9951365347f484203817b37fa41e65d52
Signed-off-by: DongHun Kwak <dh0128.kwak@samsung.com>
entities.c

index 1a8f86f..ec1b9a7 100644 (file)
@@ -112,36 +112,19 @@ xmlFreeEntity(xmlEntityPtr entity)
     if ((entity->children) && (entity->owner == 1) &&
         (entity == (xmlEntityPtr) entity->children->parent))
         xmlFreeNodeList(entity->children);
-    if (dict != NULL) {
-        if ((entity->name != NULL) && (!xmlDictOwns(dict, entity->name)))
-            xmlFree((char *) entity->name);
-        if ((entity->ExternalID != NULL) &&
-           (!xmlDictOwns(dict, entity->ExternalID)))
-            xmlFree((char *) entity->ExternalID);
-        if ((entity->SystemID != NULL) &&
-           (!xmlDictOwns(dict, entity->SystemID)))
-            xmlFree((char *) entity->SystemID);
-        if ((entity->URI != NULL) && (!xmlDictOwns(dict, entity->URI)))
-            xmlFree((char *) entity->URI);
-        if ((entity->content != NULL)
-            && (!xmlDictOwns(dict, entity->content)))
-            xmlFree((char *) entity->content);
-        if ((entity->orig != NULL) && (!xmlDictOwns(dict, entity->orig)))
-            xmlFree((char *) entity->orig);
-    } else {
-        if (entity->name != NULL)
-            xmlFree((char *) entity->name);
-        if (entity->ExternalID != NULL)
-            xmlFree((char *) entity->ExternalID);
-        if (entity->SystemID != NULL)
-            xmlFree((char *) entity->SystemID);
-        if (entity->URI != NULL)
-            xmlFree((char *) entity->URI);
-        if (entity->content != NULL)
-            xmlFree((char *) entity->content);
-        if (entity->orig != NULL)
-            xmlFree((char *) entity->orig);
-    }
+    if ((entity->name != NULL) &&
+        ((dict == NULL) || (!xmlDictOwns(dict, entity->name))))
+        xmlFree((char *) entity->name);
+    if (entity->ExternalID != NULL)
+        xmlFree((char *) entity->ExternalID);
+    if (entity->SystemID != NULL)
+        xmlFree((char *) entity->SystemID);
+    if (entity->URI != NULL)
+        xmlFree((char *) entity->URI);
+    if (entity->content != NULL)
+        xmlFree((char *) entity->content);
+    if (entity->orig != NULL)
+        xmlFree((char *) entity->orig);
     xmlFree(entity);
 }
 
@@ -177,18 +160,12 @@ xmlCreateEntity(xmlDictPtr dict, const xmlChar *name, int type,
            ret->SystemID = xmlStrdup(SystemID);
     } else {
         ret->name = xmlDictLookup(dict, name, -1);
-       if (ExternalID != NULL)
-           ret->ExternalID = xmlDictLookup(dict, ExternalID, -1);
-       if (SystemID != NULL)
-           ret->SystemID = xmlDictLookup(dict, SystemID, -1);
+       ret->ExternalID = xmlStrdup(ExternalID);
+       ret->SystemID = xmlStrdup(SystemID);
     }
     if (content != NULL) {
         ret->length = xmlStrlen(content);
-       if ((dict != NULL) && (ret->length < 5))
-           ret->content = (xmlChar *)
-                          xmlDictLookup(dict, content, ret->length);
-       else
-           ret->content = xmlStrndup(content, ret->length);
+       ret->content = xmlStrndup(content, ret->length);
      } else {
         ret->length = 0;
         ret->content = NULL;