Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 17351fc

Browse files
committed
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
1 parent 03bda45 commit 17351fc

File tree

4 files changed

+81
-2
lines changed

4 files changed

+81
-2
lines changed

src/backend/utils/adt/xml.c

+41-2
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#ifdef USE_LIBXML
4949
#include <libxml/chvalid.h>
5050
#include <libxml/parser.h>
51+
#include <libxml/parserInternals.h>
5152
#include <libxml/tree.h>
5253
#include <libxml/uri.h>
5354
#include <libxml/xmlerror.h>
@@ -99,8 +100,12 @@ struct PgXmlErrorContext
99100
/* previous libxml error handling state (saved by pg_xml_init) */
100101
xmlStructuredErrorFunc saved_errfunc;
101102
void *saved_errcxt;
103+
/* previous libxml entity handler (saved by pg_xml_init) */
104+
xmlExternalEntityLoader saved_entityfunc;
102105
};
103106

107+
static xmlParserInputPtr xmlPgEntityLoader(const char *URL, const char *ID,
108+
xmlParserCtxtPtr ctxt);
104109
static void xml_errorHandler(void *data, xmlErrorPtr error);
105110
static void xml_ereport_by_code(int level, int sqlcode,
106111
const char *msg, int errcode);
@@ -985,6 +990,13 @@ pg_xml_init(PgXmlStrictness strictness)
985990
" being used is not compatible with the libxml2"
986991
" header files that PostgreSQL was built with.")));
987992

993+
/*
994+
* Also, install an entity loader to prevent unwanted fetches of external
995+
* files and URLs.
996+
*/
997+
errcxt->saved_entityfunc = xmlGetExternalEntityLoader();
998+
xmlSetExternalEntityLoader(xmlPgEntityLoader);
999+
9881000
return errcxt;
9891001
}
9901002

@@ -1027,8 +1039,9 @@ pg_xml_done(PgXmlErrorContext *errcxt, bool isError)
10271039
if (cur_errcxt != (void *) errcxt)
10281040
elog(WARNING, "libxml error handling state is out of sync with xml.c");
10291041

1030-
/* Restore the saved handler */
1042+
/* Restore the saved handlers */
10311043
xmlSetStructuredErrorFunc(errcxt->saved_errcxt, errcxt->saved_errfunc);
1044+
xmlSetExternalEntityLoader(errcxt->saved_entityfunc);
10321045

10331046
/*
10341047
* Mark the struct as invalid, just in case somebody somehow manages to
@@ -1472,6 +1485,25 @@ xml_pstrdup(const char *string)
14721485
#endif /* USE_LIBXMLCONTEXT */
14731486

14741487

1488+
/*
1489+
* xmlPgEntityLoader --- entity loader callback function
1490+
*
1491+
* Silently prevent any external entity URL from being loaded. We don't want
1492+
* to throw an error, so instead make the entity appear to expand to an empty
1493+
* string.
1494+
*
1495+
* We would prefer to allow loading entities that exist in the system's
1496+
* global XML catalog; but the available libxml2 APIs make that a complex
1497+
* and fragile task. For now, just shut down all external access.
1498+
*/
1499+
static xmlParserInputPtr
1500+
xmlPgEntityLoader(const char *URL, const char *ID,
1501+
xmlParserCtxtPtr ctxt)
1502+
{
1503+
return xmlNewStringInputStream(ctxt, (const xmlChar *) "");
1504+
}
1505+
1506+
14751507
/*
14761508
* xml_ereport --- report an XML-related error
14771509
*
@@ -1566,7 +1598,14 @@ xml_errorHandler(void *data, xmlErrorPtr error)
15661598
case XML_FROM_NONE:
15671599
case XML_FROM_MEMORY:
15681600
case XML_FROM_IO:
1569-
/* Accept error regardless of the parsing purpose */
1601+
/*
1602+
* Suppress warnings about undeclared entities. We need to do
1603+
* this to avoid problems due to not loading DTD definitions.
1604+
*/
1605+
if (error->code == XML_WAR_UNDECLARED_ENTITY)
1606+
return;
1607+
1608+
/* Otherwise, accept error regardless of the parsing purpose */
15701609
break;
15711610

15721611
default:

src/test/regress/expected/xml.out

+20
Original file line numberDiff line numberDiff line change
@@ -896,3 +896,23 @@ CONTEXT: SQL function "xpath" statement 1
896896
{"<relativens xmlns=\"relative\"/>"}
897897
(1 row)
898898

899+
-- External entity references should not leak filesystem information.
900+
SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>');
901+
xmlparse
902+
-----------------------------------------------------------------
903+
<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>
904+
(1 row)
905+
906+
SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>');
907+
xmlparse
908+
-----------------------------------------------------------------------
909+
<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>
910+
(1 row)
911+
912+
-- This might or might not load the requested DTD, but it mustn't throw error.
913+
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>');
914+
xmlparse
915+
------------------------------------------------------------------------------------------------------------------------------------------------------
916+
<!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>
917+
(1 row)
918+

src/test/regress/expected/xml_1.out

+14
Original file line numberDiff line numberDiff line change
@@ -789,3 +789,17 @@ LINE 1: SELECT xpath('/*', '<relativens xmlns=''relative''/>');
789789
^
790790
DETAIL: This functionality requires the server to be built with libxml support.
791791
HINT: You need to rebuild PostgreSQL using --with-libxml.
792+
-- External entity references should not leak filesystem information.
793+
SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>');
794+
ERROR: unsupported XML feature
795+
DETAIL: This functionality requires the server to be built with libxml support.
796+
HINT: You need to rebuild PostgreSQL using --with-libxml.
797+
SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>');
798+
ERROR: unsupported XML feature
799+
DETAIL: This functionality requires the server to be built with libxml support.
800+
HINT: You need to rebuild PostgreSQL using --with-libxml.
801+
-- This might or might not load the requested DTD, but it mustn't throw error.
802+
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>');
803+
ERROR: unsupported XML feature
804+
DETAIL: This functionality requires the server to be built with libxml support.
805+
HINT: You need to rebuild PostgreSQL using --with-libxml.

src/test/regress/sql/xml.sql

+6
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,9 @@ SELECT xpath('/*', '<nosuchprefix:tag/>');
259259
-- XPath deprecates relative namespaces, but they're not supposed to
260260
-- throw an error, only a warning.
261261
SELECT xpath('/*', '<relativens xmlns=''relative''/>');
262+
263+
-- External entity references should not leak filesystem information.
264+
SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/passwd">]><foo>&c;</foo>');
265+
SELECT XMLPARSE(DOCUMENT '<!DOCTYPE foo [<!ENTITY c SYSTEM "/etc/no.such.file">]><foo>&c;</foo>');
266+
-- This might or might not load the requested DTD, but it mustn't throw error.
267+
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>');

0 commit comments

Comments
 (0)