public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Jim Jones <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #18943: Return value of a function 'xmlBufferCreate' is dereferenced at xpath.c:177 without checking for NUL
Date: Fri, 6 Jun 2025 14:54:40 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAPLXN34Dr3Gbi+xJ6BgCeTyBJkMVe3cn7qxoADV72rC9ZHeBtQ@mail.gmail.com>
<[email protected]>
<[email protected]>
On Thu, Jun 05, 2025 at 04:15:19PM +0200, Jim Jones wrote:
> On 05.06.25 11:47, Jim Jones wrote:
>> Taking a further look at xml.c I am wondering if other functions might
>> also need some attention in this regard:
>>
>> * xmlTextWriterStartElement [3]
>> * xmlTextWriterWriteAttribute [4]
>> * xmlTextWriterWriteRaw [5]
>> * xmlTextWriterEndAttribute [6]
It seems to me that you mean xmlTextWriterEndElement() and not
xmlTextWriterEndAttribute() for the last one.
I've been looking at the rest of the callers (this took some time),
like:
- xmlTextWriterWriteBase64, OK.
- xmlTextWriterWriteBinHex, OK.
- xmlNewStringInputStream, which is intentional in xmlPgEntityLoader()
- xmlAddChildList, where we expect valid input.
- xmlXPathCompiledEval, where valid input is expected.
- xmlXPathNewContext, which is incorrect, could fail an allocation.
- xmlReadMemory, looks OK.
- xmlBufferWriteChar, which could fail on OOM if they need to grow
memory, but let's leave these as they are; I suspect that
xmlBufferCreate() would fail anyway.
>> We're assuming they never fail. Perhaps something like this?
>> ...
>> nbytes = xmlTextWriterStartElement(writer, (xmlChar *) xexpr->name);
>> if (nbytes == -1 || xmlerrcxt->err_occurred)
>> xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
>> "could not allocate xmlTextWriterStartElement");
Oh. These can return XML_ERR_NO_MEMORY as well, with more error
patterns..
> There is also a further xmlXPathCastNodeToString() call in xml.c at
> xml_xmlnodetoxmltype() - it calls xmlNodeGetContent() and it can return
> NULL.
And it's documented as a routine that returns an allocated string, so
yes, we would miss allocation failures but we should not. I think
that we should move the call of xmlXPathCastNodeToString() inside the
PG_TRY block and rely on the xmlerrcxt given by the caller to log an
error if an allocation fails, marking xmlChar *str as volatile to free
it in the finally block if required.
> The function pgxmlNodeSetToText() also calls xmlXPathCastNodeToString(),
> but apparently xmlBufferAdd() can handle NULL values.[1]
Yeah, the patch I have posted upthread gets that done better.
What do you think?
--
Michael
Attachments:
[text/x-diff] v3-0001-xml2-Fix-error-handling-in-corner-cases.patch (22.6K, 2-v3-0001-xml2-Fix-error-handling-in-corner-cases.patch)
download | inline diff:
From 6d2e680bb75d45435b704fcf4b7225697cad4da8 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Fri, 6 Jun 2025 14:53:35 +0900
Subject: [PATCH v3] xml2: Fix error handling in corner cases
---
src/backend/utils/adt/xml.c | 73 +++++--
contrib/xml2/xpath.c | 418 ++++++++++++++++++++++++------------
contrib/xml2/xslt_proc.c | 26 ++-
3 files changed, 352 insertions(+), 165 deletions(-)
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index a4150bff2eae..43bc2432eb6c 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -529,14 +529,36 @@ xmltext(PG_FUNCTION_ARGS)
#ifdef USE_LIBXML
text *arg = PG_GETARG_TEXT_PP(0);
text *result;
- xmlChar *xmlbuf = NULL;
+ volatile xmlChar *xmlbuf = NULL;
+ PgXmlErrorContext *xmlerrcxt;
- xmlbuf = xmlEncodeSpecialChars(NULL, xml_text2xmlChar(arg));
+ /* Otherwise, we gotta spin up some error handling. */
+ xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_ALL);
- Assert(xmlbuf);
+ PG_TRY();
+ {
+ xmlbuf = xmlEncodeSpecialChars(NULL, xml_text2xmlChar(arg));
+
+ if (xmlbuf == NULL || xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate xmlChar");
+
+ result = cstring_to_text_with_len((const char *) xmlbuf,
+ xmlStrlen((const xmlChar *) xmlbuf));
+ }
+ PG_CATCH();
+ {
+ if (xmlbuf)
+ xmlFree((xmlChar *) xmlbuf);
+
+ pg_xml_done(xmlerrcxt, true);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+
+ xmlFree((xmlChar *) xmlbuf);
+ pg_xml_done(xmlerrcxt, false);
- result = cstring_to_text_with_len((const char *) xmlbuf, xmlStrlen(xmlbuf));
- xmlFree(xmlbuf);
PG_RETURN_XML_P(result);
#else
NO_XML_SUPPORT();
@@ -931,7 +953,10 @@ xmlelement(XmlExpr *xexpr,
xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not allocate xmlTextWriter");
- xmlTextWriterStartElement(writer, (xmlChar *) xexpr->name);
+ if (xmlTextWriterStartElement(writer, (xmlChar *) xexpr->name) < 0 ||
+ xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ "could not start xml element");
forboth(arg, named_arg_strings, narg, xexpr->arg_names)
{
@@ -939,19 +964,30 @@ xmlelement(XmlExpr *xexpr,
char *argname = strVal(lfirst(narg));
if (str)
- xmlTextWriterWriteAttribute(writer,
- (xmlChar *) argname,
- (xmlChar *) str);
+ {
+ if (xmlTextWriterWriteAttribute(writer,
+ (xmlChar *) argname,
+ (xmlChar *) str) < 0 ||
+ xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ "could not write xml attribute");
+ }
}
foreach(arg, arg_strings)
{
char *str = (char *) lfirst(arg);
- xmlTextWriterWriteRaw(writer, (xmlChar *) str);
+ if (xmlTextWriterWriteRaw(writer, (xmlChar *) str) < 0 ||
+ xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ "could not write raw xml text");
}
- xmlTextWriterEndElement(writer);
+ if (xmlTextWriterEndElement(writer) < 0 ||
+ xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ "could not end xml element");
/* we MUST do this now to flush data out to the buffer ... */
xmlFreeTextWriter(writer);
@@ -4220,20 +4256,27 @@ xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext *xmlerrcxt)
}
else
{
- xmlChar *str;
+ volatile xmlChar *str = NULL;
- str = xmlXPathCastNodeToString(cur);
PG_TRY();
{
+ char *escaped;
+
+ str = xmlXPathCastNodeToString(cur);
+ if (str == NULL || xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate xmlChar");
+
/* Here we rely on XML having the same representation as TEXT */
- char *escaped = escape_xml((char *) str);
+ escaped = escape_xml((char *) str);
result = (xmltype *) cstring_to_text(escaped);
pfree(escaped);
}
PG_FINALLY();
{
- xmlFree(str);
+ if (str)
+ xmlFree((xmlChar *) str);
}
PG_END_TRY();
}
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 23d3f332dbaa..9345a6c2d08f 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -51,10 +51,10 @@ static text *pgxml_result_to_text(xmlXPathObjectPtr res, xmlChar *toptag,
static xmlChar *pgxml_texttoxmlchar(text *textstring);
-static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath,
- xpath_workspace *workspace);
+static xpath_workspace *pgxml_xpath(text *document, xmlChar *xpath,
+ PgXmlErrorContext *xmlerrcxt);
-static void cleanup_workspace(xpath_workspace *workspace);
+static void cleanup_workspace(volatile xpath_workspace *workspace);
/*
@@ -89,18 +89,40 @@ xml_encode_special_chars(PG_FUNCTION_ARGS)
{
text *tin = PG_GETARG_TEXT_PP(0);
text *tout;
- xmlChar *ts,
- *tt;
+ volatile xmlChar *tt = NULL;
+ PgXmlErrorContext *xmlerrcxt;
- ts = pgxml_texttoxmlchar(tin);
+ xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_ALL);
- tt = xmlEncodeSpecialChars(NULL, ts);
+ PG_TRY();
+ {
+ xmlChar *ts;
- pfree(ts);
+ ts = pgxml_texttoxmlchar(tin);
- tout = cstring_to_text((char *) tt);
+ tt = xmlEncodeSpecialChars(NULL, ts);
+ if (tt == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate xmlChar");
+ pfree(ts);
- xmlFree(tt);
+ tout = cstring_to_text((char *) tt);
+ }
+ PG_CATCH();
+ {
+ if (tt != NULL)
+ xmlFree((xmlChar *) tt);
+
+ pg_xml_done(xmlerrcxt, true);
+
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+
+ if (tt != NULL)
+ xmlFree((xmlChar *) tt);
+
+ pg_xml_done(xmlerrcxt, false);
PG_RETURN_TEXT_P(tout);
}
@@ -122,62 +144,90 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset,
xmlChar *septagname,
xmlChar *plainsep)
{
- xmlBufferPtr buf;
+ volatile xmlBufferPtr buf = NULL;
xmlChar *result;
int i;
+ PgXmlErrorContext *xmlerrcxt;
- buf = xmlBufferCreate();
+ /* spin some error handling */
+ xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_ALL);
- if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
+ PG_TRY();
{
- xmlBufferWriteChar(buf, "<");
- xmlBufferWriteCHAR(buf, toptagname);
- xmlBufferWriteChar(buf, ">");
- }
- if (nodeset != NULL)
- {
- for (i = 0; i < nodeset->nodeNr; i++)
+ buf = xmlBufferCreate();
+
+ if (buf == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate xmlBuffer");
+
+ if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
{
- if (plainsep != NULL)
+ xmlBufferWriteChar(buf, "<");
+ xmlBufferWriteCHAR(buf, toptagname);
+ xmlBufferWriteChar(buf, ">");
+ }
+ if (nodeset != NULL)
+ {
+ for (i = 0; i < nodeset->nodeNr; i++)
{
- xmlBufferWriteCHAR(buf,
- xmlXPathCastNodeToString(nodeset->nodeTab[i]));
-
- /* If this isn't the last entry, write the plain sep. */
- if (i < (nodeset->nodeNr) - 1)
- xmlBufferWriteChar(buf, (char *) plainsep);
- }
- else
- {
- if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
+ if (plainsep != NULL)
{
- xmlBufferWriteChar(buf, "<");
- xmlBufferWriteCHAR(buf, septagname);
- xmlBufferWriteChar(buf, ">");
+ xmlBufferWriteCHAR(buf,
+ xmlXPathCastNodeToString(nodeset->nodeTab[i]));
+
+ /* If this isn't the last entry, write the plain sep. */
+ if (i < (nodeset->nodeNr) - 1)
+ xmlBufferWriteChar(buf, (char *) plainsep);
}
- xmlNodeDump(buf,
- nodeset->nodeTab[i]->doc,
- nodeset->nodeTab[i],
- 1, 0);
-
- if ((septagname != NULL) && (xmlStrlen(septagname) > 0))
+ 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))
- {
- xmlBufferWriteChar(buf, "</");
- xmlBufferWriteCHAR(buf, toptagname);
- xmlBufferWriteChar(buf, ">");
+ if ((toptagname != NULL) && (xmlStrlen(toptagname) > 0))
+ {
+ xmlBufferWriteChar(buf, "</");
+ xmlBufferWriteCHAR(buf, toptagname);
+ xmlBufferWriteChar(buf, ">");
+ }
+
+ result = xmlStrdup(buf->content);
+ if (result == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate result");
}
- result = xmlStrdup(buf->content);
+ PG_CATCH();
+ {
+ if (buf)
+ xmlBufferFree(buf);
+
+ pg_xml_done(xmlerrcxt, true);
+
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+
xmlBufferFree(buf);
+ pg_xml_done(xmlerrcxt, false);
+
return result;
}
@@ -208,16 +258,29 @@ xpath_nodeset(PG_FUNCTION_ARGS)
xmlChar *septag = pgxml_texttoxmlchar(PG_GETARG_TEXT_PP(3));
xmlChar *xpath;
text *xpres;
- xmlXPathObjectPtr res;
- xpath_workspace workspace;
+ volatile xpath_workspace *workspace;
+ PgXmlErrorContext *xmlerrcxt;
xpath = pgxml_texttoxmlchar(xpathsupp);
+ xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
- res = pgxml_xpath(document, xpath, &workspace);
+ PG_TRY();
+ {
+ workspace = pgxml_xpath(document, xpath, xmlerrcxt);
+ xpres = pgxml_result_to_text(workspace->res, toptag, septag, NULL);
+ }
+ PG_CATCH();
+ {
+ if (workspace)
+ cleanup_workspace(workspace);
- xpres = pgxml_result_to_text(res, toptag, septag, NULL);
+ pg_xml_done(xmlerrcxt, true);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
- cleanup_workspace(&workspace);
+ cleanup_workspace(workspace);
+ pg_xml_done(xmlerrcxt, false);
pfree(xpath);
@@ -240,16 +303,29 @@ xpath_list(PG_FUNCTION_ARGS)
xmlChar *plainsep = pgxml_texttoxmlchar(PG_GETARG_TEXT_PP(2));
xmlChar *xpath;
text *xpres;
- xmlXPathObjectPtr res;
- xpath_workspace workspace;
+ volatile xpath_workspace *workspace;
+ PgXmlErrorContext *xmlerrcxt;
xpath = pgxml_texttoxmlchar(xpathsupp);
+ xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
- res = pgxml_xpath(document, xpath, &workspace);
+ PG_TRY();
+ {
+ workspace = pgxml_xpath(document, xpath, xmlerrcxt);
+ xpres = pgxml_result_to_text(workspace->res, NULL, NULL, plainsep);
+ }
+ PG_CATCH();
+ {
+ if (workspace)
+ cleanup_workspace(workspace);
- xpres = pgxml_result_to_text(res, NULL, NULL, plainsep);
+ pg_xml_done(xmlerrcxt, true);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
- cleanup_workspace(&workspace);
+ cleanup_workspace(workspace);
+ pg_xml_done(xmlerrcxt, false);
pfree(xpath);
@@ -269,8 +345,8 @@ xpath_string(PG_FUNCTION_ARGS)
xmlChar *xpath;
int32 pathsize;
text *xpres;
- xmlXPathObjectPtr res;
- xpath_workspace workspace;
+ volatile xpath_workspace *workspace;
+ PgXmlErrorContext *xmlerrcxt;
pathsize = VARSIZE_ANY_EXHDR(xpathsupp);
@@ -286,11 +362,24 @@ xpath_string(PG_FUNCTION_ARGS)
xpath[pathsize + 7] = ')';
xpath[pathsize + 8] = '\0';
- res = pgxml_xpath(document, xpath, &workspace);
+ xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
- xpres = pgxml_result_to_text(res, NULL, NULL, NULL);
+ PG_TRY();
+ {
+ workspace = pgxml_xpath(document, xpath, xmlerrcxt);
+ xpres = pgxml_result_to_text(workspace->res, NULL, NULL, NULL);
+ }
+ PG_CATCH();
+ {
+ if (workspace)
+ cleanup_workspace(workspace);
- cleanup_workspace(&workspace);
+ pg_xml_done(xmlerrcxt, true);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+
+ cleanup_workspace(workspace);
pfree(xpath);
@@ -308,24 +397,38 @@ xpath_number(PG_FUNCTION_ARGS)
text *document = PG_GETARG_TEXT_PP(0);
text *xpathsupp = PG_GETARG_TEXT_PP(1); /* XPath expression */
xmlChar *xpath;
- float4 fRes;
- xmlXPathObjectPtr res;
- xpath_workspace workspace;
+ float4 fRes = 0.0;
+ bool isNull = false;
+ volatile xpath_workspace *workspace = NULL;
+ PgXmlErrorContext *xmlerrcxt;
xpath = pgxml_texttoxmlchar(xpathsupp);
+ xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
- res = pgxml_xpath(document, xpath, &workspace);
+ PG_TRY();
+ {
+ workspace = pgxml_xpath(document, xpath, xmlerrcxt);
+ pfree(xpath);
- pfree(xpath);
+ if (workspace->res == NULL)
+ isNull = true;
+ else
+ fRes = xmlXPathCastToNumber(workspace->res);
+ }
+ PG_CATCH();
+ {
+ if (workspace)
+ cleanup_workspace(workspace);
- if (res == NULL)
- PG_RETURN_NULL();
+ pg_xml_done(xmlerrcxt, true);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
- fRes = xmlXPathCastToNumber(res);
+ cleanup_workspace(workspace);
+ pg_xml_done(xmlerrcxt, false);
- cleanup_workspace(&workspace);
-
- if (xmlXPathIsNaN(fRes))
+ if (isNull || xmlXPathIsNaN(fRes))
PG_RETURN_NULL();
PG_RETURN_FLOAT4(fRes);
@@ -341,21 +444,34 @@ xpath_bool(PG_FUNCTION_ARGS)
text *xpathsupp = PG_GETARG_TEXT_PP(1); /* XPath expression */
xmlChar *xpath;
int bRes;
- xmlXPathObjectPtr res;
- xpath_workspace workspace;
+ volatile xpath_workspace *workspace = NULL;
+ PgXmlErrorContext *xmlerrcxt;
xpath = pgxml_texttoxmlchar(xpathsupp);
+ xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
- res = pgxml_xpath(document, xpath, &workspace);
+ PG_TRY();
+ {
+ workspace = pgxml_xpath(document, xpath, xmlerrcxt);
+ pfree(xpath);
- pfree(xpath);
+ if (workspace->res == NULL)
+ bRes = 0;
+ else
+ bRes = xmlXPathCastToBoolean(workspace->res);
+ }
+ PG_CATCH();
+ {
+ if (workspace)
+ cleanup_workspace(workspace);
- if (res == NULL)
- PG_RETURN_BOOL(false);
+ pg_xml_done(xmlerrcxt, true);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
- bRes = xmlXPathCastToBoolean(res);
-
- cleanup_workspace(&workspace);
+ cleanup_workspace(workspace);
+ pg_xml_done(xmlerrcxt, false);
PG_RETURN_BOOL(bRes);
}
@@ -364,62 +480,44 @@ xpath_bool(PG_FUNCTION_ARGS)
/* Core function to evaluate XPath query */
-static xmlXPathObjectPtr
-pgxml_xpath(text *document, xmlChar *xpath, xpath_workspace *workspace)
+static xpath_workspace *
+pgxml_xpath(text *document, xmlChar *xpath, PgXmlErrorContext *xmlerrcxt)
{
int32 docsize = VARSIZE_ANY_EXHDR(document);
- PgXmlErrorContext *xmlerrcxt;
xmlXPathCompExprPtr comppath;
+ xpath_workspace *workspace = (xpath_workspace *)
+ palloc0(sizeof(xpath_workspace));
workspace->doctree = NULL;
workspace->ctxt = NULL;
workspace->res = NULL;
- xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
-
- PG_TRY();
+ workspace->doctree = xmlReadMemory((char *) VARDATA_ANY(document),
+ docsize, NULL, NULL,
+ XML_PARSE_NOENT);
+ if (workspace->doctree != NULL)
{
- workspace->doctree = xmlReadMemory((char *) VARDATA_ANY(document),
- docsize, NULL, NULL,
- XML_PARSE_NOENT);
- if (workspace->doctree != NULL)
- {
- workspace->ctxt = xmlXPathNewContext(workspace->doctree);
- workspace->ctxt->node = xmlDocGetRootElement(workspace->doctree);
+ workspace->ctxt = xmlXPathNewContext(workspace->doctree);
+ workspace->ctxt->node = xmlDocGetRootElement(workspace->doctree);
- /* compile the path */
- comppath = xmlXPathCtxtCompile(workspace->ctxt, xpath);
- if (comppath == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
+ /* compile the path */
+ comppath = xmlXPathCtxtCompile(workspace->ctxt, xpath);
+ if (comppath == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"XPath Syntax Error");
- /* Now evaluate the path expression. */
- workspace->res = xmlXPathCompiledEval(comppath, workspace->ctxt);
+ /* Now evaluate the path expression. */
+ workspace->res = xmlXPathCompiledEval(comppath, workspace->ctxt);
- xmlXPathFreeCompExpr(comppath);
- }
+ xmlXPathFreeCompExpr(comppath);
}
- PG_CATCH();
- {
- cleanup_workspace(workspace);
- pg_xml_done(xmlerrcxt, true);
-
- PG_RE_THROW();
- }
- PG_END_TRY();
-
- if (workspace->res == NULL)
- cleanup_workspace(workspace);
-
- pg_xml_done(xmlerrcxt, false);
-
- return workspace->res;
+ return workspace;
}
/* Clean up after processing the result of pgxml_xpath() */
static void
-cleanup_workspace(xpath_workspace *workspace)
+cleanup_workspace(volatile xpath_workspace *workspace)
{
if (workspace->res)
xmlXPathFreeObject(workspace->res);
@@ -438,34 +536,59 @@ pgxml_result_to_text(xmlXPathObjectPtr res,
xmlChar *septag,
xmlChar *plainsep)
{
- xmlChar *xpresstr;
+ volatile xmlChar *xpresstr = NULL;
+ PgXmlErrorContext *xmlerrcxt;
text *xpres;
if (res == NULL)
return NULL;
- switch (res->type)
+ /* spin some error handling */
+ xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_ALL);
+
+ PG_TRY();
{
- case XPATH_NODESET:
- xpresstr = pgxmlNodeSetToText(res->nodesetval,
- toptag,
- septag, plainsep);
- break;
+ switch (res->type)
+ {
+ case XPATH_NODESET:
+ xpresstr = pgxmlNodeSetToText(res->nodesetval,
+ toptag,
+ septag, plainsep);
+ break;
- case XPATH_STRING:
- xpresstr = xmlStrdup(res->stringval);
- break;
+ case XPATH_STRING:
+ xpresstr = xmlStrdup(res->stringval);
+ if (xpresstr == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate result");
+ break;
- default:
- elog(NOTICE, "unsupported XQuery result: %d", res->type);
- xpresstr = xmlStrdup((const xmlChar *) "<unsupported/>");
+ default:
+ elog(NOTICE, "unsupported XQuery result: %d", res->type);
+ xpresstr = xmlStrdup((const xmlChar *) "<unsupported/>");
+ if (xpresstr == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate result");
+ }
+
+ /* Now convert this result back to text */
+ xpres = cstring_to_text((char *) xpresstr);
}
+ PG_CATCH();
+ {
+ if (xpresstr != NULL)
+ xmlFree((xmlChar *) xpresstr);
- /* Now convert this result back to text */
- xpres = cstring_to_text((char *) xpresstr);
+ pg_xml_done(xmlerrcxt, true);
+
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
/* Free various storage */
- xmlFree(xpresstr);
+ xmlFree((xmlChar *) xpresstr);
+
+ pg_xml_done(xmlerrcxt, false);
return xpres;
}
@@ -648,11 +771,16 @@ xpath_table(PG_FUNCTION_ARGS)
for (j = 0; j < numpaths; j++)
{
ctxt = xmlXPathNewContext(doctree);
+ if (ctxt == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt,
+ ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate XPath context");
+
ctxt->node = xmlDocGetRootElement(doctree);
/* compile the path */
comppath = xmlXPathCtxtCompile(ctxt, xpaths[j]);
- if (comppath == NULL)
+ if (comppath == NULL || pg_xml_error_occurred(xmlerrcxt))
xml_ereport(xmlerrcxt, ERROR,
ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"XPath Syntax Error");
@@ -671,6 +799,10 @@ xpath_table(PG_FUNCTION_ARGS)
rownr < res->nodesetval->nodeNr)
{
resstr = xmlXPathCastNodeToString(res->nodesetval->nodeTab[rownr]);
+ if (resstr == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt,
+ ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate result");
had_values = true;
}
else
@@ -680,11 +812,19 @@ xpath_table(PG_FUNCTION_ARGS)
case XPATH_STRING:
resstr = xmlStrdup(res->stringval);
+ if (resstr == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt,
+ ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate result");
break;
default:
elog(NOTICE, "unsupported XQuery result: %d", res->type);
resstr = xmlStrdup((const xmlChar *) "<unsupported/>");
+ if (resstr == NULL || pg_xml_error_occurred(xmlerrcxt))
+ xml_ereport(xmlerrcxt,
+ ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate result");
}
/*
diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c
index b720d89f754a..c8e7dd45ed5b 100644
--- a/contrib/xml2/xslt_proc.c
+++ b/contrib/xml2/xslt_proc.c
@@ -58,7 +58,7 @@ xslt_process(PG_FUNCTION_ARGS)
volatile xsltSecurityPrefsPtr xslt_sec_prefs = NULL;
volatile xsltTransformContextPtr xslt_ctxt = NULL;
volatile int resstat = -1;
- xmlChar *resstr = NULL;
+ volatile xmlChar *resstr = NULL;
int reslen = 0;
if (fcinfo->nargs == 3)
@@ -86,7 +86,7 @@ xslt_process(PG_FUNCTION_ARGS)
VARSIZE_ANY_EXHDR(doct), NULL, NULL,
XML_PARSE_NOENT);
- if (doctree == NULL)
+ if (doctree == NULL || pg_xml_error_occurred(xmlerrcxt))
xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing XML document");
@@ -95,14 +95,14 @@ xslt_process(PG_FUNCTION_ARGS)
VARSIZE_ANY_EXHDR(ssheet), NULL, NULL,
XML_PARSE_NOENT);
- if (ssdoc == NULL)
+ if (ssdoc == NULL || pg_xml_error_occurred(xmlerrcxt))
xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing stylesheet as XML document");
/* After this call we need not free ssdoc separately */
stylesheet = xsltParseStylesheetDoc(ssdoc);
- if (stylesheet == NULL)
+ if (stylesheet == NULL || pg_xml_error_occurred(xmlerrcxt))
xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"failed to parse stylesheet");
@@ -137,11 +137,15 @@ xslt_process(PG_FUNCTION_ARGS)
restree = xsltApplyStylesheetUser(stylesheet, doctree, params,
NULL, NULL, xslt_ctxt);
- if (restree == NULL)
+ if (restree == NULL || pg_xml_error_occurred(xmlerrcxt))
xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"failed to apply stylesheet");
- resstat = xsltSaveResultToString(&resstr, &reslen, restree, stylesheet);
+ resstat = xsltSaveResultToString((xmlChar **) &resstr, &reslen,
+ restree, stylesheet);
+
+ if (resstat >= 0)
+ result = cstring_to_text_with_len((char *) resstr, reslen);
}
PG_CATCH();
{
@@ -155,6 +159,8 @@ xslt_process(PG_FUNCTION_ARGS)
xsltFreeStylesheet(stylesheet);
if (doctree != NULL)
xmlFreeDoc(doctree);
+ if (resstr != NULL)
+ xmlFree((xmlChar *) resstr);
xsltCleanupGlobals();
pg_xml_done(xmlerrcxt, true);
@@ -170,17 +176,15 @@ xslt_process(PG_FUNCTION_ARGS)
xmlFreeDoc(doctree);
xsltCleanupGlobals();
+ if (resstr)
+ xmlFree((xmlChar *) resstr);
+
pg_xml_done(xmlerrcxt, false);
/* XXX this is pretty dubious, really ought to throw error instead */
if (resstat < 0)
PG_RETURN_NULL();
- result = cstring_to_text_with_len((char *) resstr, reslen);
-
- if (resstr)
- xmlFree(resstr);
-
PG_RETURN_TEXT_P(result);
#else /* !USE_LIBXSLT */
--
2.49.0
[application/pgp-signature] signature.asc (833B, 3-signature.asc)
download
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #18943: Return value of a function 'xmlBufferCreate' is dereferenced at xpath.c:177 without checking for NUL
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox