1From 8d22e065888942b8c1b5be8994c6887b5a687246 Mon Sep 17 00:00:00 2001 2From: Nick Wellnhofer <wellnhofer@aevum.de> 3Date: Wed, 15 Feb 2023 14:41:11 +0100 4Subject: [PATCH] malloc-fail: Fix memory leak after calling 5 xmlXPathNodeSetMerge 6 7Destroy the first argument in xmlXPathNodeSetMerge if the function 8fails. This is somewhat dangerous but matches the expectations of users. 9 10Found with libFuzzer, see #344. 11 12Reference:https://github.com/GNOME/libxml2/commit/8d22e065888942b8c1b5be8994c6887b5a687246 13Conflict:xpath.c 14--- 15 xpath.c | 78 +++++++++++++++++++++++++++------------------------------ 16 1 file changed, 37 insertions(+), 41 deletions(-) 17 18diff --git a/xpath.c b/xpath.c 19index 9ead497..5a6d762 100644 20--- a/xpath.c 21+++ b/xpath.c 22@@ -153,6 +153,9 @@ 23 * any use of the macros IS_ASCII_CHARACTER and IS_ASCII_DIGIT) 24 */ 25 26+static void 27+xmlXPathNodeSetClear(xmlNodeSetPtr set, int hasNsNodes); 28+ 29 #ifdef XP_OPTIMIZED_NON_ELEM_COMPARISON 30 /** 31 * xmlXPathCmpNodesExt: 32@@ -3840,6 +3843,8 @@ xmlXPathNodeSetAddUnique(xmlNodeSetPtr cur, xmlNodePtr val) { 33 * if @val1 is NULL, a new set is created and copied from @val2 34 * 35 * Returns @val1 once extended or NULL in case of error. 36+ * 37+ * Frees @val1 in case of error. 38 */ 39 xmlNodeSetPtr 40 xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 41@@ -3849,35 +3854,8 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 42 if (val2 == NULL) return(val1); 43 if (val1 == NULL) { 44 val1 = xmlXPathNodeSetCreate(NULL); 45- if (val1 == NULL) 46- return (NULL); 47-#if 0 48- /* 49- * TODO: The optimization won't work in every case, since 50- * those nasty namespace nodes need to be added with 51- * xmlXPathNodeSetDupNs() to the set; thus a pure 52- * memcpy is not possible. 53- * If there was a flag on the nodesetval, indicating that 54- * some temporary nodes are in, that would be helpful. 55- */ 56- /* 57- * Optimization: Create an equally sized node-set 58- * and memcpy the content. 59- */ 60- val1 = xmlXPathNodeSetCreateSize(val2->nodeNr); 61- if (val1 == NULL) 62- return(NULL); 63- if (val2->nodeNr != 0) { 64- if (val2->nodeNr == 1) 65- *(val1->nodeTab) = *(val2->nodeTab); 66- else { 67- memcpy(val1->nodeTab, val2->nodeTab, 68- val2->nodeNr * sizeof(xmlNodePtr)); 69- } 70- val1->nodeNr = val2->nodeNr; 71- } 72- return(val1); 73-#endif 74+ if (val1 == NULL) 75+ return (NULL); 76 } 77 78 /* @@ with_ns to check whether namespace nodes should be looked at @@ */ 79@@ -3916,7 +3894,7 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 80 sizeof(xmlNodePtr)); 81 if (val1->nodeTab == NULL) { 82 xmlXPathErrMemory(NULL, "merging nodeset\n"); 83- return(NULL); 84+ goto error; 85 } 86 memset(val1->nodeTab, 0 , 87 XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); 88@@ -3926,13 +3904,13 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 89 90 if (val1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { 91 xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); 92- return(NULL); 93+ goto error; 94 } 95 temp = (xmlNodePtr *) xmlRealloc(val1->nodeTab, val1->nodeMax * 2 * 96 sizeof(xmlNodePtr)); 97 if (temp == NULL) { 98 xmlXPathErrMemory(NULL, "merging nodeset\n"); 99- return(NULL); 100+ goto error; 101 } 102 val1->nodeTab = temp; 103 val1->nodeMax *= 2; 104@@ -3942,13 +3920,17 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 105 xmlNodePtr nsNode = xmlXPathNodeSetDupNs((xmlNodePtr) ns->next, ns); 106 107 if (nsNode == NULL) 108- return(NULL); 109+ goto error; 110 val1->nodeTab[val1->nodeNr++] = nsNode; 111 } else 112 val1->nodeTab[val1->nodeNr++] = n2; 113 } 114 115 return(val1); 116+ 117+error: 118+ xmlXPathFreeNodeSet(val1); 119+ return(NULL); 120 } 121 122 123@@ -3961,6 +3943,8 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 124 * Checks for duplicate nodes. Clears set2. 125 * 126 * Returns @set1 once extended or NULL in case of error. 127+ * 128+ * Frees @set1 in case of error. 129 */ 130 static xmlNodeSetPtr 131 xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 132@@ -3989,7 +3973,6 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 133 /* 134 * Free the namespace node. 135 */ 136- set2->nodeTab[i] = NULL; 137 xmlXPathNodeSetFreeNs((xmlNsPtr) n2); 138 goto skip_node; 139 } 140@@ -4003,7 +3986,7 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 141 XML_NODESET_DEFAULT * sizeof(xmlNodePtr)); 142 if (set1->nodeTab == NULL) { 143 xmlXPathErrMemory(NULL, "merging nodeset\n"); 144- return(NULL); 145+ goto error; 146 } 147 memset(set1->nodeTab, 0, 148 XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); 149@@ -4013,24 +3996,29 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 150 151 if (set1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { 152 xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); 153- return(NULL); 154+ goto error; 155 } 156 temp = (xmlNodePtr *) xmlRealloc( 157 set1->nodeTab, set1->nodeMax * 2 * sizeof(xmlNodePtr)); 158 if (temp == NULL) { 159 xmlXPathErrMemory(NULL, "merging nodeset\n"); 160- return(NULL); 161+ goto error; 162 } 163 set1->nodeTab = temp; 164 set1->nodeMax *= 2; 165 } 166 set1->nodeTab[set1->nodeNr++] = n2; 167 skip_node: 168- {} 169+ set2->nodeTab[i] = NULL; 170 } 171 } 172 set2->nodeNr = 0; 173 return(set1); 174+ 175+error: 176+ xmlXPathFreeNodeSet(set1); 177+ xmlXPathNodeSetClear(set2, 1); 178+ return(NULL); 179 } 180 181 /** 182@@ -4042,6 +4030,8 @@ skip_node: 183 * Doesn't check for duplicate nodes. Clears set2. 184 * 185 * Returns @set1 once extended or NULL in case of error. 186+ * 187+ * Frees @set1 in case of error. 188 */ 189 static xmlNodeSetPtr 190 xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 191@@ -4057,7 +4047,7 @@ xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 192 XML_NODESET_DEFAULT * sizeof(xmlNodePtr)); 193 if (set1->nodeTab == NULL) { 194 xmlXPathErrMemory(NULL, "merging nodeset\n"); 195- return(NULL); 196+ goto error; 197 } 198 memset(set1->nodeTab, 0, 199 XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); 200@@ -4067,22 +4057,28 @@ xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 201 202 if (set1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { 203 xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); 204- return(NULL); 205+ goto error; 206 } 207 temp = (xmlNodePtr *) xmlRealloc( 208 set1->nodeTab, set1->nodeMax * 2 * sizeof(xmlNodePtr)); 209 if (temp == NULL) { 210 xmlXPathErrMemory(NULL, "merging nodeset\n"); 211- return(NULL); 212+ goto error; 213 } 214 set1->nodeTab = temp; 215 set1->nodeMax *= 2; 216 } 217 set1->nodeTab[set1->nodeNr++] = n2; 218+ set2->nodeTab[i] = NULL; 219 } 220 } 221 set2->nodeNr = 0; 222 return(set1); 223+ 224+error: 225+ xmlXPathFreeNodeSet(set1); 226+ xmlXPathNodeSetClear(set2, 1); 227+ return(NULL); 228 } 229 230 /** 231-- 2322.27.0 233 234