153aa9179Sopenharmony_ciFrom 1b41ec4e9433b05bb0376be4725804c54ef1d80b Mon Sep 17 00:00:00 2001
253aa9179Sopenharmony_ciFrom: Nick Wellnhofer <wellnhofer@aevum.de>
353aa9179Sopenharmony_ciDate: Wed, 31 Aug 2022 22:11:25 +0200
453aa9179Sopenharmony_ciSubject: [PATCH] [CVE-2022-40304] Fix dict corruption caused by entity
553aa9179Sopenharmony_ci reference cycles
653aa9179Sopenharmony_ci
753aa9179Sopenharmony_ciWhen an entity reference cycle is detected, the entity content is
853aa9179Sopenharmony_cicleared by setting its first byte to zero. But the entity content might
953aa9179Sopenharmony_cibe allocated from a dict. In this case, the dict entry becomes corrupted
1053aa9179Sopenharmony_cileading to all kinds of logic errors, including memory errors like
1153aa9179Sopenharmony_cidouble-frees.
1253aa9179Sopenharmony_ci
1353aa9179Sopenharmony_ciStop storing entity content, orig, ExternalID and SystemID in a dict.
1453aa9179Sopenharmony_ciThese values are unlikely to occur multiple times in a document, so they
1553aa9179Sopenharmony_cishouldn't have been stored in a dict in the first place.
1653aa9179Sopenharmony_ci
1753aa9179Sopenharmony_ciThanks to Ned Williamson and Nathan Wachholz working with Google Project
1853aa9179Sopenharmony_ciZero for the report!
1953aa9179Sopenharmony_ci---
2053aa9179Sopenharmony_ci entities.c | 55 ++++++++++++++++--------------------------------------
2153aa9179Sopenharmony_ci 1 file changed, 16 insertions(+), 39 deletions(-)
2253aa9179Sopenharmony_ci
2353aa9179Sopenharmony_cidiff --git a/entities.c b/entities.c
2453aa9179Sopenharmony_ciindex 84435515..d4e5412e 100644
2553aa9179Sopenharmony_ci--- a/entities.c
2653aa9179Sopenharmony_ci+++ b/entities.c
2753aa9179Sopenharmony_ci@@ -128,36 +128,19 @@ xmlFreeEntity(xmlEntityPtr entity)
2853aa9179Sopenharmony_ci     if ((entity->children) && (entity->owner == 1) &&
2953aa9179Sopenharmony_ci         (entity == (xmlEntityPtr) entity->children->parent))
3053aa9179Sopenharmony_ci         xmlFreeNodeList(entity->children);
3153aa9179Sopenharmony_ci-    if (dict != NULL) {
3253aa9179Sopenharmony_ci-        if ((entity->name != NULL) && (!xmlDictOwns(dict, entity->name)))
3353aa9179Sopenharmony_ci-            xmlFree((char *) entity->name);
3453aa9179Sopenharmony_ci-        if ((entity->ExternalID != NULL) &&
3553aa9179Sopenharmony_ci-	    (!xmlDictOwns(dict, entity->ExternalID)))
3653aa9179Sopenharmony_ci-            xmlFree((char *) entity->ExternalID);
3753aa9179Sopenharmony_ci-        if ((entity->SystemID != NULL) &&
3853aa9179Sopenharmony_ci-	    (!xmlDictOwns(dict, entity->SystemID)))
3953aa9179Sopenharmony_ci-            xmlFree((char *) entity->SystemID);
4053aa9179Sopenharmony_ci-        if ((entity->URI != NULL) && (!xmlDictOwns(dict, entity->URI)))
4153aa9179Sopenharmony_ci-            xmlFree((char *) entity->URI);
4253aa9179Sopenharmony_ci-        if ((entity->content != NULL)
4353aa9179Sopenharmony_ci-            && (!xmlDictOwns(dict, entity->content)))
4453aa9179Sopenharmony_ci-            xmlFree((char *) entity->content);
4553aa9179Sopenharmony_ci-        if ((entity->orig != NULL) && (!xmlDictOwns(dict, entity->orig)))
4653aa9179Sopenharmony_ci-            xmlFree((char *) entity->orig);
4753aa9179Sopenharmony_ci-    } else {
4853aa9179Sopenharmony_ci-        if (entity->name != NULL)
4953aa9179Sopenharmony_ci-            xmlFree((char *) entity->name);
5053aa9179Sopenharmony_ci-        if (entity->ExternalID != NULL)
5153aa9179Sopenharmony_ci-            xmlFree((char *) entity->ExternalID);
5253aa9179Sopenharmony_ci-        if (entity->SystemID != NULL)
5353aa9179Sopenharmony_ci-            xmlFree((char *) entity->SystemID);
5453aa9179Sopenharmony_ci-        if (entity->URI != NULL)
5553aa9179Sopenharmony_ci-            xmlFree((char *) entity->URI);
5653aa9179Sopenharmony_ci-        if (entity->content != NULL)
5753aa9179Sopenharmony_ci-            xmlFree((char *) entity->content);
5853aa9179Sopenharmony_ci-        if (entity->orig != NULL)
5953aa9179Sopenharmony_ci-            xmlFree((char *) entity->orig);
6053aa9179Sopenharmony_ci-    }
6153aa9179Sopenharmony_ci+    if ((entity->name != NULL) &&
6253aa9179Sopenharmony_ci+        ((dict == NULL) || (!xmlDictOwns(dict, entity->name))))
6353aa9179Sopenharmony_ci+        xmlFree((char *) entity->name);
6453aa9179Sopenharmony_ci+    if (entity->ExternalID != NULL)
6553aa9179Sopenharmony_ci+        xmlFree((char *) entity->ExternalID);
6653aa9179Sopenharmony_ci+    if (entity->SystemID != NULL)
6753aa9179Sopenharmony_ci+        xmlFree((char *) entity->SystemID);
6853aa9179Sopenharmony_ci+    if (entity->URI != NULL)
6953aa9179Sopenharmony_ci+        xmlFree((char *) entity->URI);
7053aa9179Sopenharmony_ci+    if (entity->content != NULL)
7153aa9179Sopenharmony_ci+        xmlFree((char *) entity->content);
7253aa9179Sopenharmony_ci+    if (entity->orig != NULL)
7353aa9179Sopenharmony_ci+        xmlFree((char *) entity->orig);
7453aa9179Sopenharmony_ci     xmlFree(entity);
7553aa9179Sopenharmony_ci }
7653aa9179Sopenharmony_ci 
7753aa9179Sopenharmony_ci@@ -193,18 +176,12 @@ xmlCreateEntity(xmlDictPtr dict, const xmlChar *name, int type,
7853aa9179Sopenharmony_ci 	    ret->SystemID = xmlStrdup(SystemID);
7953aa9179Sopenharmony_ci     } else {
8053aa9179Sopenharmony_ci         ret->name = xmlDictLookup(dict, name, -1);
8153aa9179Sopenharmony_ci-	if (ExternalID != NULL)
8253aa9179Sopenharmony_ci-	    ret->ExternalID = xmlDictLookup(dict, ExternalID, -1);
8353aa9179Sopenharmony_ci-	if (SystemID != NULL)
8453aa9179Sopenharmony_ci-	    ret->SystemID = xmlDictLookup(dict, SystemID, -1);
8553aa9179Sopenharmony_ci+	ret->ExternalID = xmlStrdup(ExternalID);
8653aa9179Sopenharmony_ci+	ret->SystemID = xmlStrdup(SystemID);
8753aa9179Sopenharmony_ci     }
8853aa9179Sopenharmony_ci     if (content != NULL) {
8953aa9179Sopenharmony_ci         ret->length = xmlStrlen(content);
9053aa9179Sopenharmony_ci-	if ((dict != NULL) && (ret->length < 5))
9153aa9179Sopenharmony_ci-	    ret->content = (xmlChar *)
9253aa9179Sopenharmony_ci-	                   xmlDictLookup(dict, content, ret->length);
9353aa9179Sopenharmony_ci-	else
9453aa9179Sopenharmony_ci-	    ret->content = xmlStrndup(content, ret->length);
9553aa9179Sopenharmony_ci+	ret->content = xmlStrndup(content, ret->length);
9653aa9179Sopenharmony_ci      } else {
9753aa9179Sopenharmony_ci         ret->length = 0;
9853aa9179Sopenharmony_ci         ret->content = NULL;
9953aa9179Sopenharmony_ci-- 
10053aa9179Sopenharmony_ci2.27.0
10153aa9179Sopenharmony_ci
102