Greetings, everyone!

While analyzing output of Svace static analyzer [1] I've found a bug.

In function pgxmlNodeSetToText there is a call of xmlBufferCreate that doesn't have its return value checked. In all four other calls of xmlBufferCreate there
is a try...catch that checks the return value inside.

I suggest to add the same checks here that are used in other four calls of
xmlBufferCreate.

The proposed patch is attached.

[1] - https://svace.pages.ispras.ru/svace-website/en/

Oleg Tselebrovskiy, Postgres Pro
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 94641930f7b..2d72ade9c20 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -122,62 +122,85 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset,
 				   xmlChar *septagname,
 				   xmlChar *plainsep)
 {
-	xmlBufferPtr buf;
+	xmlBufferPtr buf = NULL;
 	xmlChar    *result;
 	int			i;
+	PgXmlErrorContext *xmlerrcxt;
 
-	buf = xmlBufferCreate();
+	xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_LEGACY);
 
-	if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
-	{
-		xmlBufferWriteChar(buf, "<");
-		xmlBufferWriteCHAR(buf, toptagname);
-		xmlBufferWriteChar(buf, ">");
-	}
-	if (nodeset != NULL)
+	PG_TRY();
 	{
-		for (i = 0; i < nodeset->nodeNr; i++)
-		{
-			if (plainsep != NULL)
-			{
-				xmlBufferWriteCHAR(buf,
-								   xmlXPathCastNodeToString(nodeset->nodeTab[i]));
+		buf = xmlBufferCreate();
 
-				/* If this isn't the last entry, write the plain sep. */
-				if (i < (nodeset->nodeNr) - 1)
-					xmlBufferWriteChar(buf, (char *) plainsep);
-			}
-			else
+		if (buf == NULL)
+			xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+						"could not allocate xmlBuffer");
+
+		if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
+		{
+			xmlBufferWriteChar(buf, "<");
+			xmlBufferWriteCHAR(buf, toptagname);
+			xmlBufferWriteChar(buf, ">");
+		}
+		if (nodeset != NULL)
+		{
+			for (i = 0; i < nodeset->nodeNr; i++)
 			{
-				if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
+				if (plainsep != NULL)
 				{
-					xmlBufferWriteChar(buf, "<");
-					xmlBufferWriteCHAR(buf, septagname);
-					xmlBufferWriteChar(buf, ">");
-				}
-				xmlNodeDump(buf,
-							nodeset->nodeTab[i]->doc,
-							nodeset->nodeTab[i],
-							1, 0);
+					xmlBufferWriteCHAR(buf,
+									xmlXPathCastNodeToString(nodeset->nodeTab[i]));
 
-				if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
+					/* If this isn't the last entry, write the plain sep. */
+					if (i < (nodeset->nodeNr) - 1)
+						xmlBufferWriteChar(buf, (char *) plainsep);
+				}
+				else
 				{
-					xmlBufferWriteChar(buf, "</");
-					xmlBufferWriteCHAR(buf, septagname);
-					xmlBufferWriteChar(buf, ">");
+					if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
+					{
+						xmlBufferWriteChar(buf, "<");
+						xmlBufferWriteCHAR(buf, septagname);
+						xmlBufferWriteChar(buf, ">");
+					}
+					xmlNodeDump(buf,
+								nodeset->nodeTab[i]->doc,
+								nodeset->nodeTab[i],
+								1, 0);
+
+					if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
+					{
+						xmlBufferWriteChar(buf, "</");
+						xmlBufferWriteCHAR(buf, septagname);
+						xmlBufferWriteChar(buf, ">");
+					}
 				}
 			}
 		}
-	}
 
-	if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
+		if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
+		{
+			xmlBufferWriteChar(buf, "</");
+			xmlBufferWriteCHAR(buf, toptagname);
+			xmlBufferWriteChar(buf, ">");
+		}
+		result = xmlStrdup(buf->content);
+	}
+	PG_CATCH();
 	{
-		xmlBufferWriteChar(buf, "</");
-		xmlBufferWriteCHAR(buf, toptagname);
-		xmlBufferWriteChar(buf, ">");
+		if (buf)
+			xmlBufferFree(buf);
+
+		pg_xml_done(xmlerrcxt, true);
+
+		PG_RE_THROW();
 	}
-	result = xmlStrdup(buf->content);
+	PG_END_TRY();
+
 	xmlBufferFree(buf);
+	pg_xml_done(xmlerrcxt, false);
+
 	return result;
 }
 

Reply via email to