153aa9179Sopenharmony_ciFrom 8d22e065888942b8c1b5be8994c6887b5a687246 Mon Sep 17 00:00:00 2001 253aa9179Sopenharmony_ciFrom: Nick Wellnhofer <wellnhofer@aevum.de> 353aa9179Sopenharmony_ciDate: Wed, 15 Feb 2023 14:41:11 +0100 453aa9179Sopenharmony_ciSubject: [PATCH] malloc-fail: Fix memory leak after calling 553aa9179Sopenharmony_ci xmlXPathNodeSetMerge 653aa9179Sopenharmony_ci 753aa9179Sopenharmony_ciDestroy the first argument in xmlXPathNodeSetMerge if the function 853aa9179Sopenharmony_cifails. This is somewhat dangerous but matches the expectations of users. 953aa9179Sopenharmony_ci 1053aa9179Sopenharmony_ciFound with libFuzzer, see #344. 1153aa9179Sopenharmony_ci 1253aa9179Sopenharmony_ciReference:https://github.com/GNOME/libxml2/commit/8d22e065888942b8c1b5be8994c6887b5a687246 1353aa9179Sopenharmony_ciConflict:xpath.c 1453aa9179Sopenharmony_ci--- 1553aa9179Sopenharmony_ci xpath.c | 78 +++++++++++++++++++++++++++------------------------------ 1653aa9179Sopenharmony_ci 1 file changed, 37 insertions(+), 41 deletions(-) 1753aa9179Sopenharmony_ci 1853aa9179Sopenharmony_cidiff --git a/xpath.c b/xpath.c 1953aa9179Sopenharmony_ciindex 9ead497..5a6d762 100644 2053aa9179Sopenharmony_ci--- a/xpath.c 2153aa9179Sopenharmony_ci+++ b/xpath.c 2253aa9179Sopenharmony_ci@@ -153,6 +153,9 @@ 2353aa9179Sopenharmony_ci * any use of the macros IS_ASCII_CHARACTER and IS_ASCII_DIGIT) 2453aa9179Sopenharmony_ci */ 2553aa9179Sopenharmony_ci 2653aa9179Sopenharmony_ci+static void 2753aa9179Sopenharmony_ci+xmlXPathNodeSetClear(xmlNodeSetPtr set, int hasNsNodes); 2853aa9179Sopenharmony_ci+ 2953aa9179Sopenharmony_ci #ifdef XP_OPTIMIZED_NON_ELEM_COMPARISON 3053aa9179Sopenharmony_ci /** 3153aa9179Sopenharmony_ci * xmlXPathCmpNodesExt: 3253aa9179Sopenharmony_ci@@ -3840,6 +3843,8 @@ xmlXPathNodeSetAddUnique(xmlNodeSetPtr cur, xmlNodePtr val) { 3353aa9179Sopenharmony_ci * if @val1 is NULL, a new set is created and copied from @val2 3453aa9179Sopenharmony_ci * 3553aa9179Sopenharmony_ci * Returns @val1 once extended or NULL in case of error. 3653aa9179Sopenharmony_ci+ * 3753aa9179Sopenharmony_ci+ * Frees @val1 in case of error. 3853aa9179Sopenharmony_ci */ 3953aa9179Sopenharmony_ci xmlNodeSetPtr 4053aa9179Sopenharmony_ci xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 4153aa9179Sopenharmony_ci@@ -3849,35 +3854,8 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 4253aa9179Sopenharmony_ci if (val2 == NULL) return(val1); 4353aa9179Sopenharmony_ci if (val1 == NULL) { 4453aa9179Sopenharmony_ci val1 = xmlXPathNodeSetCreate(NULL); 4553aa9179Sopenharmony_ci- if (val1 == NULL) 4653aa9179Sopenharmony_ci- return (NULL); 4753aa9179Sopenharmony_ci-#if 0 4853aa9179Sopenharmony_ci- /* 4953aa9179Sopenharmony_ci- * TODO: The optimization won't work in every case, since 5053aa9179Sopenharmony_ci- * those nasty namespace nodes need to be added with 5153aa9179Sopenharmony_ci- * xmlXPathNodeSetDupNs() to the set; thus a pure 5253aa9179Sopenharmony_ci- * memcpy is not possible. 5353aa9179Sopenharmony_ci- * If there was a flag on the nodesetval, indicating that 5453aa9179Sopenharmony_ci- * some temporary nodes are in, that would be helpful. 5553aa9179Sopenharmony_ci- */ 5653aa9179Sopenharmony_ci- /* 5753aa9179Sopenharmony_ci- * Optimization: Create an equally sized node-set 5853aa9179Sopenharmony_ci- * and memcpy the content. 5953aa9179Sopenharmony_ci- */ 6053aa9179Sopenharmony_ci- val1 = xmlXPathNodeSetCreateSize(val2->nodeNr); 6153aa9179Sopenharmony_ci- if (val1 == NULL) 6253aa9179Sopenharmony_ci- return(NULL); 6353aa9179Sopenharmony_ci- if (val2->nodeNr != 0) { 6453aa9179Sopenharmony_ci- if (val2->nodeNr == 1) 6553aa9179Sopenharmony_ci- *(val1->nodeTab) = *(val2->nodeTab); 6653aa9179Sopenharmony_ci- else { 6753aa9179Sopenharmony_ci- memcpy(val1->nodeTab, val2->nodeTab, 6853aa9179Sopenharmony_ci- val2->nodeNr * sizeof(xmlNodePtr)); 6953aa9179Sopenharmony_ci- } 7053aa9179Sopenharmony_ci- val1->nodeNr = val2->nodeNr; 7153aa9179Sopenharmony_ci- } 7253aa9179Sopenharmony_ci- return(val1); 7353aa9179Sopenharmony_ci-#endif 7453aa9179Sopenharmony_ci+ if (val1 == NULL) 7553aa9179Sopenharmony_ci+ return (NULL); 7653aa9179Sopenharmony_ci } 7753aa9179Sopenharmony_ci 7853aa9179Sopenharmony_ci /* @@ with_ns to check whether namespace nodes should be looked at @@ */ 7953aa9179Sopenharmony_ci@@ -3916,7 +3894,7 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 8053aa9179Sopenharmony_ci sizeof(xmlNodePtr)); 8153aa9179Sopenharmony_ci if (val1->nodeTab == NULL) { 8253aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset\n"); 8353aa9179Sopenharmony_ci- return(NULL); 8453aa9179Sopenharmony_ci+ goto error; 8553aa9179Sopenharmony_ci } 8653aa9179Sopenharmony_ci memset(val1->nodeTab, 0 , 8753aa9179Sopenharmony_ci XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); 8853aa9179Sopenharmony_ci@@ -3926,13 +3904,13 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 8953aa9179Sopenharmony_ci 9053aa9179Sopenharmony_ci if (val1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { 9153aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); 9253aa9179Sopenharmony_ci- return(NULL); 9353aa9179Sopenharmony_ci+ goto error; 9453aa9179Sopenharmony_ci } 9553aa9179Sopenharmony_ci temp = (xmlNodePtr *) xmlRealloc(val1->nodeTab, val1->nodeMax * 2 * 9653aa9179Sopenharmony_ci sizeof(xmlNodePtr)); 9753aa9179Sopenharmony_ci if (temp == NULL) { 9853aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset\n"); 9953aa9179Sopenharmony_ci- return(NULL); 10053aa9179Sopenharmony_ci+ goto error; 10153aa9179Sopenharmony_ci } 10253aa9179Sopenharmony_ci val1->nodeTab = temp; 10353aa9179Sopenharmony_ci val1->nodeMax *= 2; 10453aa9179Sopenharmony_ci@@ -3942,13 +3920,17 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 10553aa9179Sopenharmony_ci xmlNodePtr nsNode = xmlXPathNodeSetDupNs((xmlNodePtr) ns->next, ns); 10653aa9179Sopenharmony_ci 10753aa9179Sopenharmony_ci if (nsNode == NULL) 10853aa9179Sopenharmony_ci- return(NULL); 10953aa9179Sopenharmony_ci+ goto error; 11053aa9179Sopenharmony_ci val1->nodeTab[val1->nodeNr++] = nsNode; 11153aa9179Sopenharmony_ci } else 11253aa9179Sopenharmony_ci val1->nodeTab[val1->nodeNr++] = n2; 11353aa9179Sopenharmony_ci } 11453aa9179Sopenharmony_ci 11553aa9179Sopenharmony_ci return(val1); 11653aa9179Sopenharmony_ci+ 11753aa9179Sopenharmony_ci+error: 11853aa9179Sopenharmony_ci+ xmlXPathFreeNodeSet(val1); 11953aa9179Sopenharmony_ci+ return(NULL); 12053aa9179Sopenharmony_ci } 12153aa9179Sopenharmony_ci 12253aa9179Sopenharmony_ci 12353aa9179Sopenharmony_ci@@ -3961,6 +3943,8 @@ xmlXPathNodeSetMerge(xmlNodeSetPtr val1, xmlNodeSetPtr val2) { 12453aa9179Sopenharmony_ci * Checks for duplicate nodes. Clears set2. 12553aa9179Sopenharmony_ci * 12653aa9179Sopenharmony_ci * Returns @set1 once extended or NULL in case of error. 12753aa9179Sopenharmony_ci+ * 12853aa9179Sopenharmony_ci+ * Frees @set1 in case of error. 12953aa9179Sopenharmony_ci */ 13053aa9179Sopenharmony_ci static xmlNodeSetPtr 13153aa9179Sopenharmony_ci xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 13253aa9179Sopenharmony_ci@@ -3989,7 +3973,6 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 13353aa9179Sopenharmony_ci /* 13453aa9179Sopenharmony_ci * Free the namespace node. 13553aa9179Sopenharmony_ci */ 13653aa9179Sopenharmony_ci- set2->nodeTab[i] = NULL; 13753aa9179Sopenharmony_ci xmlXPathNodeSetFreeNs((xmlNsPtr) n2); 13853aa9179Sopenharmony_ci goto skip_node; 13953aa9179Sopenharmony_ci } 14053aa9179Sopenharmony_ci@@ -4003,7 +3986,7 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 14153aa9179Sopenharmony_ci XML_NODESET_DEFAULT * sizeof(xmlNodePtr)); 14253aa9179Sopenharmony_ci if (set1->nodeTab == NULL) { 14353aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset\n"); 14453aa9179Sopenharmony_ci- return(NULL); 14553aa9179Sopenharmony_ci+ goto error; 14653aa9179Sopenharmony_ci } 14753aa9179Sopenharmony_ci memset(set1->nodeTab, 0, 14853aa9179Sopenharmony_ci XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); 14953aa9179Sopenharmony_ci@@ -4013,24 +3996,29 @@ xmlXPathNodeSetMergeAndClear(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 15053aa9179Sopenharmony_ci 15153aa9179Sopenharmony_ci if (set1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { 15253aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); 15353aa9179Sopenharmony_ci- return(NULL); 15453aa9179Sopenharmony_ci+ goto error; 15553aa9179Sopenharmony_ci } 15653aa9179Sopenharmony_ci temp = (xmlNodePtr *) xmlRealloc( 15753aa9179Sopenharmony_ci set1->nodeTab, set1->nodeMax * 2 * sizeof(xmlNodePtr)); 15853aa9179Sopenharmony_ci if (temp == NULL) { 15953aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset\n"); 16053aa9179Sopenharmony_ci- return(NULL); 16153aa9179Sopenharmony_ci+ goto error; 16253aa9179Sopenharmony_ci } 16353aa9179Sopenharmony_ci set1->nodeTab = temp; 16453aa9179Sopenharmony_ci set1->nodeMax *= 2; 16553aa9179Sopenharmony_ci } 16653aa9179Sopenharmony_ci set1->nodeTab[set1->nodeNr++] = n2; 16753aa9179Sopenharmony_ci skip_node: 16853aa9179Sopenharmony_ci- {} 16953aa9179Sopenharmony_ci+ set2->nodeTab[i] = NULL; 17053aa9179Sopenharmony_ci } 17153aa9179Sopenharmony_ci } 17253aa9179Sopenharmony_ci set2->nodeNr = 0; 17353aa9179Sopenharmony_ci return(set1); 17453aa9179Sopenharmony_ci+ 17553aa9179Sopenharmony_ci+error: 17653aa9179Sopenharmony_ci+ xmlXPathFreeNodeSet(set1); 17753aa9179Sopenharmony_ci+ xmlXPathNodeSetClear(set2, 1); 17853aa9179Sopenharmony_ci+ return(NULL); 17953aa9179Sopenharmony_ci } 18053aa9179Sopenharmony_ci 18153aa9179Sopenharmony_ci /** 18253aa9179Sopenharmony_ci@@ -4042,6 +4030,8 @@ skip_node: 18353aa9179Sopenharmony_ci * Doesn't check for duplicate nodes. Clears set2. 18453aa9179Sopenharmony_ci * 18553aa9179Sopenharmony_ci * Returns @set1 once extended or NULL in case of error. 18653aa9179Sopenharmony_ci+ * 18753aa9179Sopenharmony_ci+ * Frees @set1 in case of error. 18853aa9179Sopenharmony_ci */ 18953aa9179Sopenharmony_ci static xmlNodeSetPtr 19053aa9179Sopenharmony_ci xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 19153aa9179Sopenharmony_ci@@ -4057,7 +4047,7 @@ xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 19253aa9179Sopenharmony_ci XML_NODESET_DEFAULT * sizeof(xmlNodePtr)); 19353aa9179Sopenharmony_ci if (set1->nodeTab == NULL) { 19453aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset\n"); 19553aa9179Sopenharmony_ci- return(NULL); 19653aa9179Sopenharmony_ci+ goto error; 19753aa9179Sopenharmony_ci } 19853aa9179Sopenharmony_ci memset(set1->nodeTab, 0, 19953aa9179Sopenharmony_ci XML_NODESET_DEFAULT * (size_t) sizeof(xmlNodePtr)); 20053aa9179Sopenharmony_ci@@ -4067,22 +4057,28 @@ xmlXPathNodeSetMergeAndClearNoDupls(xmlNodeSetPtr set1, xmlNodeSetPtr set2) 20153aa9179Sopenharmony_ci 20253aa9179Sopenharmony_ci if (set1->nodeMax >= XPATH_MAX_NODESET_LENGTH) { 20353aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset hit limit\n"); 20453aa9179Sopenharmony_ci- return(NULL); 20553aa9179Sopenharmony_ci+ goto error; 20653aa9179Sopenharmony_ci } 20753aa9179Sopenharmony_ci temp = (xmlNodePtr *) xmlRealloc( 20853aa9179Sopenharmony_ci set1->nodeTab, set1->nodeMax * 2 * sizeof(xmlNodePtr)); 20953aa9179Sopenharmony_ci if (temp == NULL) { 21053aa9179Sopenharmony_ci xmlXPathErrMemory(NULL, "merging nodeset\n"); 21153aa9179Sopenharmony_ci- return(NULL); 21253aa9179Sopenharmony_ci+ goto error; 21353aa9179Sopenharmony_ci } 21453aa9179Sopenharmony_ci set1->nodeTab = temp; 21553aa9179Sopenharmony_ci set1->nodeMax *= 2; 21653aa9179Sopenharmony_ci } 21753aa9179Sopenharmony_ci set1->nodeTab[set1->nodeNr++] = n2; 21853aa9179Sopenharmony_ci+ set2->nodeTab[i] = NULL; 21953aa9179Sopenharmony_ci } 22053aa9179Sopenharmony_ci } 22153aa9179Sopenharmony_ci set2->nodeNr = 0; 22253aa9179Sopenharmony_ci return(set1); 22353aa9179Sopenharmony_ci+ 22453aa9179Sopenharmony_ci+error: 22553aa9179Sopenharmony_ci+ xmlXPathFreeNodeSet(set1); 22653aa9179Sopenharmony_ci+ xmlXPathNodeSetClear(set2, 1); 22753aa9179Sopenharmony_ci+ return(NULL); 22853aa9179Sopenharmony_ci } 22953aa9179Sopenharmony_ci 23053aa9179Sopenharmony_ci /** 23153aa9179Sopenharmony_ci-- 23253aa9179Sopenharmony_ci2.27.0 23353aa9179Sopenharmony_ci 234