Prevent access to external files/URLs via XML entity references.

xml_parse() would attempt to fetch external files or URLs as needed to
resolve DTD and entity references in an XML value, thus allowing
unprivileged database users to attempt to fetch data with the privileges
of the database server.  While the external data wouldn't get returned
directly to the user, portions of it could be exposed in error messages
if the data didn't parse as valid XML; and in any case the mere ability
to check existence of a file might be useful to an attacker.

The ideal solution to this would still allow fetching of references that
are listed in the host system's XML catalogs, so that documents can be
validated according to installed DTDs.  However, doing that with the
available libxml2 APIs appears complex and error-prone, so we're not going
to risk it in a security patch that necessarily hasn't gotten wide review.
So this patch merely shuts off all access, causing any external fetch to
silently expand to an empty string.  A future patch may improve this.

In HEAD and 9.2, also suppress warnings about undefined entities, which
would otherwise occur as a result of not loading referenced DTDs.  Previous
branches don't show such warnings anyway, due to different error handling
arrangements.

Credit to Noah Misch for first reporting the problem, and for much work
towards a solution, though this simplistic approach was not his preference.
Also thanks to Daniel Veillard for consultation.

Security: CVE-2012-3489

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index a9150e856921c33496a036a9fa8a0c622f47178d..a358de68728fbb5b4dba125824b68f07e0842f55 100644 (file)
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -48,6 +48,7 @@
 #ifdef USE_LIBXML
 #include <libxml/chvalid.h>
 #include <libxml/parser.h>
+#include <libxml/parserInternals.h>
 #include <libxml/tree.h>
 #include <libxml/uri.h>
 #include <libxml/xmlerror.h>
@@ -99,8 +100,12 @@ struct PgXmlErrorContext
    /* previous libxml error handling state (saved by pg_xml_init) */
    xmlStructuredErrorFunc saved_errfunc;
    void       *saved_errcxt;
+   /* previous libxml entity handler (saved by pg_xml_init) */
+   xmlExternalEntityLoader saved_entityfunc;
 };
 
+static xmlParserInputPtr xmlPgEntityLoader(const char *URL, const char *ID,
+                 xmlParserCtxtPtr ctxt);
 static void xml_errorHandler(void *data, xmlErrorPtr error);
 static void xml_ereport_by_code(int level, int sqlcode,
                    const char *msg, int errcode);
@@ -985,6 +990,13 @@ pg_xml_init(PgXmlStrictness strictness)
                         " being used is not compatible with the libxml2"
                         " header files that PostgreSQL was built with.")));
 
+   /*
+    * Also, install an entity loader to prevent unwanted fetches of external
+    * files and URLs.
+    */
+   errcxt->saved_entityfunc = xmlGetExternalEntityLoader();
+   xmlSetExternalEntityLoader(xmlPgEntityLoader);
+
    return errcxt;
 }
 
@@ -1027,8 +1039,9 @@ pg_xml_done(PgXmlErrorContext *errcxt, bool isError)
    if (cur_errcxt != (void *) errcxt)
        elog(WARNING, "libxml error handling state is out of sync with xml.c");
 
-   /* Restore the saved handler */
+   /* Restore the saved handlers */
    xmlSetStructuredErrorFunc(errcxt->saved_errcxt, errcxt->saved_errfunc);
+   xmlSetExternalEntityLoader(errcxt->saved_entityfunc);
 
    /*
     * Mark the struct as invalid, just in case somebody somehow manages to
@@ -1472,6 +1485,25 @@ xml_pstrdup(const char *string)
 #endif   /* USE_LIBXMLCONTEXT */
 
 
+/*
+ * xmlPgEntityLoader --- entity loader callback function
+ *
+ * Silently prevent any external entity URL from being loaded.  We don't want
+ * to throw an error, so instead make the entity appear to expand to an empty
+ * string.
+ *
+ * We would prefer to allow loading entities that exist in the system's
+ * global XML catalog; but the available libxml2 APIs make that a complex
+ * and fragile task.  For now, just shut down all external access.
+ */
+static xmlParserInputPtr
+xmlPgEntityLoader(const char *URL, const char *ID,
+                 xmlParserCtxtPtr ctxt)
+{
+   return xmlNewStringInputStream(ctxt, (const xmlChar *) "");
+}
+
+
 /*
  * xml_ereport --- report an XML-related error
  *
@@ -1566,7 +1598,14 @@ xml_errorHandler(void *data, xmlErrorPtr error)
        case XML_FROM_NONE:
        case XML_FROM_MEMORY:
        case XML_FROM_IO:
-           /* Accept error regardless of the parsing purpose */
+           /*
+            * Suppress warnings about undeclared entities.  We need to do
+            * this to avoid problems due to not loading DTD definitions.
+            */
+           if (error->code == XML_WAR_UNDECLARED_ENTITY)
+               return;
+
+           /* Otherwise, accept error regardless of the parsing purpose */
            break;
 
        default:

diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 5dfa44b5b999d8b155ca93783d3d6352f993763a..4f1e3972dcb2c8acca273ab27de5d8ba2174b781 100644 (file)
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -896,3 +896,23 @@ CONTEXT:  SQL function "xpath" statement 1
  {"<relativens xmlns=\"relative\"/>"}
 (1 row)
 
+-- External entity references should not leak filesystem information.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>');
+                            xmlparse                             
+-----------------------------------------------------------------
+ <!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>
+(1 row)
+
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>');
+                               xmlparse                                
+-----------------------------------------------------------------------
+ <!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>
+(1 row)
+
+-- This might or might not load the requested DTD, but it mustn't throw error.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN" "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd"><chapter>&nbsp;</chapter>');
+                                                                       xmlparse                                                                       
+------------------------------------------------------------------------------------------------------------------------------------------------------
+ <!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN" "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd"><chapter>&nbsp;</chapter>
+(1 row)
+

diff --git a/src/test/regress/expected/xml_1.out b/src/test/regress/expected/xml_1.out
index c6c0e7ac887d3f4790e7cc75266dcbc8f63f2745..044c5529ef23bff38fdf64b307c6c578578bed7d 100644 (file)
--- a/src/test/regress/expected/xml_1.out
+++ b/src/test/regress/expected/xml_1.out
@@ -789,3 +789,17 @@ LINE 1: SELECT xpath('/*', '<relativens xmlns=''relative''/>');
                            ^
 DETAIL:  This functionality requires the server to be built with libxml support.
 HINT:  You need to rebuild PostgreSQL using --with-libxml.
+-- External entity references should not leak filesystem information.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>');
+ERROR:  unsupported XML feature
+DETAIL:  This functionality requires the server to be built with libxml support.
+HINT:  You need to rebuild PostgreSQL using --with-libxml.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>');
+ERROR:  unsupported XML feature
+DETAIL:  This functionality requires the server to be built with libxml support.
+HINT:  You need to rebuild PostgreSQL using --with-libxml.
+-- This might or might not load the requested DTD, but it mustn't throw error.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN" "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd"><chapter>&nbsp;</chapter>');
+ERROR:  unsupported XML feature
+DETAIL:  This functionality requires the server to be built with libxml support.
+HINT:  You need to rebuild PostgreSQL using --with-libxml.

diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index 3623dbc254efd9b2d8d82d1ee703fdfedf2da5bc..90d4d67f04e289f9bee569c7c8eb349881d840a1 100644 (file)
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -259,3 +259,9 @@ SELECT xpath('/*', '<nosuchprefix:tag/>');
 -- XPath deprecates relative namespaces, but they're not supposed to
 -- throw an error, only a warning.
 SELECT xpath('/*', '<relativens xmlns=''relative''/>');
+
+-- External entity references should not leak filesystem information.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>');
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>');
+-- This might or might not load the requested DTD, but it mustn't throw error.
+SELECT XMLPARSE(DOCUMENT '<!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN" "http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd"><chapter>&nbsp;</chapter>');