From 2fbf7876510dd9c5996151e2569078146e869697 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Wed, 2 Nov 2022 16:22:54 +0100 Subject: [PATCH 12/28] malloc-fail: Fix memory leak in xmlStringGetNodeList Also make sure to return NULL on error instead of a partial node list. Found with libFuzzer, see #344. Reference: https://github.com/GNOME/libxml2/commit/b45927095e0c857b68a96466e3075d60a6a5dd9e Conflict: NA --- tree.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tree.c b/tree.c index bb85220..ac156e1 100644 --- a/tree.c +++ b/tree.c @@ -1496,9 +1496,9 @@ out: */ xmlNodePtr xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { - xmlNodePtr ret = NULL, last = NULL; + xmlNodePtr ret = NULL, head = NULL, last = NULL; xmlNodePtr node; - xmlChar *val; + xmlChar *val = NULL; const xmlChar *cur = value; const xmlChar *q; xmlEntityPtr ent; @@ -1596,14 +1596,12 @@ xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { */ if (!xmlBufIsEmpty(buf)) { node = xmlNewDocText(doc, NULL); - if (node == NULL) { - if (val != NULL) xmlFree(val); - goto out; - } + if (node == NULL) + goto out; node->content = xmlBufDetach(buf); if (last == NULL) { - last = ret = node; + last = head = node; } else { last = xmlAddNextSibling(last, node); } @@ -1613,11 +1611,9 @@ xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { * Create a new REFERENCE_REF node */ node = xmlNewReference(doc, val); - if (node == NULL) { - if (val != NULL) xmlFree(val); + if (node == NULL) goto out; - } - else if ((ent != NULL) && (ent->children == NULL)) { + if ((ent != NULL) && (ent->children == NULL)) { xmlNodePtr temp; /* Set to non-NULL value to avoid recursion. */ @@ -1633,12 +1629,13 @@ xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { } } if (last == NULL) { - last = ret = node; + last = head = node; } else { last = xmlAddNextSibling(last, node); } } xmlFree(val); + val = NULL; } cur++; q = cur; @@ -1657,7 +1654,7 @@ xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { } else cur++; } - if ((cur != q) || (ret == NULL)) { + if ((cur != q) || (head == NULL)) { /* * Handle the last piece of text. */ @@ -1666,21 +1663,24 @@ xmlStringGetNodeList(const xmlDoc *doc, const xmlChar *value) { if (!xmlBufIsEmpty(buf)) { node = xmlNewDocText(doc, NULL); - if (node == NULL) { - xmlBufFree(buf); - return(NULL); - } + if (node == NULL) + goto out; node->content = xmlBufDetach(buf); if (last == NULL) { - ret = node; + head = node; } else { xmlAddNextSibling(last, node); } } + ret = head; + head = NULL; + out: xmlBufFree(buf); + if (val != NULL) xmlFree(val); + if (head != NULL) xmlFreeNodeList(head); return(ret); } -- 2.27.0