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;
}
