Improve error handling of libxml2 calls in xml.c

This commit fixes some defects in the backend's xml.c, found upon
inspection of the internals of libxml2:
- xmlEncodeSpecialChars() can fail on malloc(), returning NULL back to
the caller.  xmltext() assumed that this could never happen.  Like other
code paths, a TRY/CATCH block is added there, covering also the fact
that cstring_to_text_with_len() could fail a memory allocation, where
the backend would miss to free the buffer allocated by
xmlEncodeSpecialChars().
- Some libxml2 routines called in xmlelement() can return NULL, like
xmlAddChildList() or xmlTextWriterStartElement().  Dedicated errors are
added for them.
- xml_xmlnodetoxmltype() missed that xmlXPathCastNodeToString() can fail
on an allocation failure.  In this case, the call can just be moved to
the existing TRY/CATCH block.

All these code paths would cause the server to crash.  As this is
unlikely a problem in practice, no backpatch is done.  Jim and I have
caught these defects, not sure who has scored the most.  The contrib
module xml2/ has similar defects, which will be addressed in a separate
change.

Reported-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/aEEingzOta_S_Nu7@paquier.xyz

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index a4150bff2eaea8f560e48ecf495d2c46d40d5526..2bd39b6ac4b09861f4a17f7a10a7c4bbb822454a 100644 (file)
--- 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;
+
+       /* Otherwise, we gotta spin up some error handling. */
+       xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_ALL);
 
-       xmlbuf = xmlEncodeSpecialChars(NULL, xml_text2xmlChar(arg));
+       PG_TRY();
+       {
+               xmlbuf = xmlEncodeSpecialChars(NULL, xml_text2xmlChar(arg));
 
-       Assert(xmlbuf);
+               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();
@@ -770,7 +792,10 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
                        if (oldroot != NULL)
                                xmlFreeNode(oldroot);
 
-                       xmlAddChildList(root, content_nodes);
+                       if (xmlAddChildList(root, content_nodes) == NULL ||
+                               xmlerrcxt->err_occurred)
+                               xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+                                                       "could not append xml node list");
 
                        /*
                         * We use this node to insert newlines in the dump.  Note: in at
@@ -931,7 +956,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 +967,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 +4259,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();
        }