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