From 8d22e065888942b8c1b5be8994c6887b5a687246 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Wed, 15 Feb 2023 14:41:11 +0100 Subject: [PATCH] malloc-fail: Fix memory leak after calling xmlXPathNodeSetMerge Destroy the first argument in xmlXPathNodeSetMerge if the function fails. This is somewhat dangerous but matches the expectations of users. Found with libFuzzer, see #344. Reference:https://github.com/GNOME/libxml2/commit/8d22e065888942b8c1b5be8994c6887b5a687246 Conflict:xpath.c --- xpath.c | 78 +++++++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/xpath.c b/xpath.c index 9ead497..5a6d762 100644 --- a/xpath.c +++ b/xpath.c @@ -153,6 +153,9 @@ * any use of the macros IS_ASCII_CHARACTER and IS_ASCII_DIGIT) */ +static void +xmlXPathNodeSetClear(xmlNodeSetPtr set, int hasNsNodes); + #ifdef XP_OPTIMIZED_NON_ELEM_COMPARISON /** * xmlXPathCmpNodesExt: @@ -3840,6 +3843,8 @@ xmlXPathNodeSetAddUnique(xmlNodeSetPtr cur, xmlNodePtr val) { * if @val1 is NULL, a new set is created and copied from @val2 * * Returns @val1 once extended or NULL in case of error. + * + * Frees @val1 in case of error. */ xmlNodeSetPtr xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { @@ -3849,35 +3854,8 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { if (val2 == NULL) return(val1); if (val1 == NULL) { val1 = xmlXPathNodeSetCreate(NULL); - if (val1 == NULL) - return (NULL); -#if 0 - /* - * TODO: The optimization won't work in every case, since - * those nasty namespace nodes need to be added with - * xmlXPathNodeSetDupNs() to the set; thus a pure - * memcpy is not possible. - * If there was a flag on the nodesetval, indicating that - * some temporary nodes are in, that would be helpful. - */ - /* - * Optimization: Create an equally sized node-set - * and memcpy the content. - */ - val1 = xmlXPathNodeSetCreateSize(val2->nodeNr); - if (val1 == NULL) - return(NULL); - if (val2->nodeNr != 0) { - if (val2->nodeNr == 1) - *(val1->nodeTab) = *(val2->nodeTab); - else { - memcpy(val1->nodeTab, val2->nodeTab, - val2->nodeNr * sizeof(xmlNodePtr)); - } - val1->nodeNr = val2->nodeNr; - } - return(val1); -#endif + if (val1 == NULL) + return (NULL); } /* @@ with_ns to check whether namespace nodes should be looked at @@ */ @@ -3916,7 +3894,7 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { sizeof(xmlNodePtr)); if (val1->nodeTab == NULL) { xmlXPathErrMemory(NULL, "merging nodeset\n"); - return(NULL); + goto error; } memset(val1->nodeTab, 0 , XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); @@ -3926,13 +3904,13 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { if (val1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); - return(NULL); + goto error; } temp = (xmlNodePtr *) xmlRealloc(val1->nodeTab, val1->nodeMax * 2 * sizeof(xmlNodePtr)); if (temp == NULL) { xmlXPathErrMemory(NULL, "merging nodeset\n"); - return(NULL); + goto error; } val1->nodeTab = temp; val1->nodeMax *= 2; @@ -3942,13 +3920,17 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { xmlNodePtr nsNode = xmlXPathNodeSetDupNs((xmlNodePtr) ns->next, ns); if (nsNode == NULL) - return(NULL); + goto error; val1->nodeTab[val1->nodeNr++] = nsNode; } else val1->nodeTab[val1->nodeNr++] = n2; } return(val1); + +error: + xmlXPathFreeNodeSet(val1); + return(NULL); } @@ -3961,6 +3943,8 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { * Checks for duplicate nodes. Clears set2. * * Returns @set1 once extended or NULL in case of error. + * + * Frees @set1 in case of error. */ static xmlNodeSetPtr xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) @@ -3989,7 +3973,6 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) /* * Free the namespace node. */ - set2->nodeTab[i] = NULL; xmlXPathNodeSetFreeNs((xmlNsPtr) n2); goto skip_node; } @@ -4003,7 +3986,7 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) XML_NODESET_DEFAULT * sizeof(xmlNodePtr)); if (set1->nodeTab == NULL) { xmlXPathErrMemory(NULL, "merging nodeset\n"); - return(NULL); + goto error; } memset(set1->nodeTab, 0, XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); @@ -4013,24 +3996,29 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) if (set1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); - return(NULL); + goto error; } temp = (xmlNodePtr *) xmlRealloc( set1->nodeTab, set1->nodeMax * 2 * sizeof(xmlNodePtr)); if (temp == NULL) { xmlXPathErrMemory(NULL, "merging nodeset\n"); - return(NULL); + goto error; } set1->nodeTab = temp; set1->nodeMax *= 2; } set1->nodeTab[set1->nodeNr++] = n2; skip_node: - {} + set2->nodeTab[i] = NULL; } } set2->nodeNr = 0; return(set1); + +error: + xmlXPathFreeNodeSet(set1); + xmlXPathNodeSetClear(set2, 1); + return(NULL); } /** @@ -4042,6 +4030,8 @@ skip_node: * Doesn't check for duplicate nodes. Clears set2. * * Returns @set1 once extended or NULL in case of error. + * + * Frees @set1 in case of error. */ static xmlNodeSetPtr xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) @@ -4057,7 +4047,7 @@ xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) XML_NODESET_DEFAULT * sizeof(xmlNodePtr)); if (set1->nodeTab == NULL) { xmlXPathErrMemory(NULL, "merging nodeset\n"); - return(NULL); + goto error; } memset(set1->nodeTab, 0, XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); @@ -4067,22 +4057,28 @@ xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) if (set1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); - return(NULL); + goto error; } temp = (xmlNodePtr *) xmlRealloc( set1->nodeTab, set1->nodeMax * 2 * sizeof(xmlNodePtr)); if (temp == NULL) { xmlXPathErrMemory(NULL, "merging nodeset\n"); - return(NULL); + goto error; } set1->nodeTab = temp; set1->nodeMax *= 2; } set1->nodeTab[set1->nodeNr++] = n2; + set2->nodeTab[i] = NULL; } } set2->nodeNr = 0; return(set1); + +error: + xmlXPathFreeNodeSet(set1); + xmlXPathNodeSetClear(set2, 1); + return(NULL); } /** -- 2.27.0