From ac7e13d6fc6fa211f2e4582a959ead762d15c997 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 14 Aug 2012 18:28:48 -0400 Subject: [PATCH] Prevent access to external files/URLs via contrib/xml2's xslt_process(). libxslt offers the ability to read and write both files and URLs through stylesheet commands, thus allowing unprivileged database users to both read and write data with the privileges of the database server. Disable that through proper use of libxslt's security options. Also, remove xslt_process()'s ability to fetch documents and stylesheets from external files/URLs. While this was a documented "feature", it was long regarded as a terrible idea. The fix for CVE-2012-3489 broke that capability, and rather than expend effort on trying to fix it, we're just going to summarily remove it. While the ability to write as well as read makes this security hole considerably worse than CVE-2012-3489, the problem is mitigated by the fact that xslt_process() is not available unless contrib/xml2 is installed, and the longstanding warnings about security risks from that should have discouraged prudent DBAs from installing it in security-exposed databases. Reported and fixed by Peter Eisentraut. Security: CVE-2012-3488 --- contrib/xml2/expected/xml2.out | 15 ++++++ contrib/xml2/expected/xml2_1.out | 15 ++++++ contrib/xml2/sql/xml2.sql | 15 ++++++ contrib/xml2/xslt_proc.c | 79 +++++++++++++++++++++++--------- doc/src/sgml/xml2.sgml | 8 ---- 5 files changed, 103 insertions(+), 29 deletions(-) diff --git a/contrib/xml2/expected/xml2.out b/contrib/xml2/expected/xml2.out index 74896b0802..90b7472dc7 100644 --- a/contrib/xml2/expected/xml2.out +++ b/contrib/xml2/expected/xml2.out @@ -145,3 +145,18 @@ values Value'); create index idx_xpath on t1 ( xpath_string ('/attributes/attribute[@name="attr_1"]/text()', xml_data::text)); +-- possible security exploit +SELECT xslt_process('Hello from XML', +$$ + + + + + + + +$$); +ERROR: failed to apply stylesheet diff --git a/contrib/xml2/expected/xml2_1.out b/contrib/xml2/expected/xml2_1.out index 083fc3b2ca..1a7433c446 100644 --- a/contrib/xml2/expected/xml2_1.out +++ b/contrib/xml2/expected/xml2_1.out @@ -107,3 +107,18 @@ values Value'); create index idx_xpath on t1 ( xpath_string ('/attributes/attribute[@name="attr_1"]/text()', xml_data::text)); +-- possible security exploit +SELECT xslt_process('Hello from XML', +$$ + + + + + + + +$$); +ERROR: xslt_process() is not available without libxslt diff --git a/contrib/xml2/sql/xml2.sql b/contrib/xml2/sql/xml2.sql index 73723b6be1..71d3535d14 100644 --- a/contrib/xml2/sql/xml2.sql +++ b/contrib/xml2/sql/xml2.sql @@ -80,3 +80,18 @@ Value'); create index idx_xpath on t1 ( xpath_string ('/attributes/attribute[@name="attr_1"]/text()', xml_data::text)); + +-- possible security exploit +SELECT xslt_process('Hello from XML', +$$ + + + + + + + +$$); diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c index 90232eb0a1..6e4bf55051 100644 --- a/contrib/xml2/xslt_proc.c +++ b/contrib/xml2/xslt_proc.c @@ -26,6 +26,7 @@ #include #include +#include #include #include #endif /* USE_LIBXSLT */ @@ -62,7 +63,10 @@ xslt_process(PG_FUNCTION_ARGS) xsltStylesheetPtr stylesheet = NULL; xmlDocPtr doctree; xmlDocPtr restree; - xmlDocPtr ssdoc = NULL; + xmlDocPtr ssdoc; + xsltSecurityPrefsPtr xslt_sec_prefs; + bool xslt_sec_prefs_error; + xsltTransformContextPtr xslt_ctxt; xmlChar *resstr; int resstat; int reslen; @@ -79,34 +83,27 @@ xslt_process(PG_FUNCTION_ARGS) /* Setup parser */ pgxml_parser_init(); - /* Check to see if document is a file or a literal */ - - if (VARDATA(doct)[0] == '<') - doctree = xmlParseMemory((char *) VARDATA(doct), VARSIZE(doct) - VARHDRSZ); - else - doctree = xmlParseFile(text_to_cstring(doct)); + /* Parse document */ + doctree = xmlParseMemory((char *) VARDATA(doct), + VARSIZE(doct) - VARHDRSZ); if (doctree == NULL) xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION, "error parsing XML document"); /* Same for stylesheet */ - if (VARDATA(ssheet)[0] == '<') - { - ssdoc = xmlParseMemory((char *) VARDATA(ssheet), - VARSIZE(ssheet) - VARHDRSZ); - if (ssdoc == NULL) - { - xmlFreeDoc(doctree); - xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION, - "error parsing stylesheet as XML document"); - } + ssdoc = xmlParseMemory((char *) VARDATA(ssheet), + VARSIZE(ssheet) - VARHDRSZ); - stylesheet = xsltParseStylesheetDoc(ssdoc); + if (ssdoc == NULL) + { + xmlFreeDoc(doctree); + xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION, + "error parsing stylesheet as XML document"); } - else - stylesheet = xsltParseStylesheetFile((xmlChar *) text_to_cstring(ssheet)); + /* After this call we need not free ssdoc separately */ + stylesheet = xsltParseStylesheetDoc(ssdoc); if (stylesheet == NULL) { @@ -116,12 +113,50 @@ xslt_process(PG_FUNCTION_ARGS) "failed to parse stylesheet"); } - restree = xsltApplyStylesheet(stylesheet, doctree, params); + xslt_ctxt = xsltNewTransformContext(stylesheet, doctree); + + xslt_sec_prefs_error = false; + if ((xslt_sec_prefs = xsltNewSecurityPrefs()) == NULL) + xslt_sec_prefs_error = true; + + if (xsltSetSecurityPrefs(xslt_sec_prefs, XSLT_SECPREF_READ_FILE, + xsltSecurityForbid) != 0) + xslt_sec_prefs_error = true; + if (xsltSetSecurityPrefs(xslt_sec_prefs, XSLT_SECPREF_WRITE_FILE, + xsltSecurityForbid) != 0) + xslt_sec_prefs_error = true; + if (xsltSetSecurityPrefs(xslt_sec_prefs, XSLT_SECPREF_CREATE_DIRECTORY, + xsltSecurityForbid) != 0) + xslt_sec_prefs_error = true; + if (xsltSetSecurityPrefs(xslt_sec_prefs, XSLT_SECPREF_READ_NETWORK, + xsltSecurityForbid) != 0) + xslt_sec_prefs_error = true; + if (xsltSetSecurityPrefs(xslt_sec_prefs, XSLT_SECPREF_WRITE_NETWORK, + xsltSecurityForbid) != 0) + xslt_sec_prefs_error = true; + if (xsltSetCtxtSecurityPrefs(xslt_sec_prefs, xslt_ctxt) != 0) + xslt_sec_prefs_error = true; + + if (xslt_sec_prefs_error) + { + xsltFreeStylesheet(stylesheet); + xmlFreeDoc(doctree); + xsltFreeSecurityPrefs(xslt_sec_prefs); + xsltFreeTransformContext(xslt_ctxt); + xsltCleanupGlobals(); + xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION, + "could not set libxslt security preferences"); + } + + restree = xsltApplyStylesheetUser(stylesheet, doctree, params, + NULL, NULL, xslt_ctxt); if (restree == NULL) { xsltFreeStylesheet(stylesheet); xmlFreeDoc(doctree); + xsltFreeSecurityPrefs(xslt_sec_prefs); + xsltFreeTransformContext(xslt_ctxt); xsltCleanupGlobals(); xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION, "failed to apply stylesheet"); @@ -132,6 +167,8 @@ xslt_process(PG_FUNCTION_ARGS) xsltFreeStylesheet(stylesheet); xmlFreeDoc(restree); xmlFreeDoc(doctree); + xsltFreeSecurityPrefs(xslt_sec_prefs); + xsltFreeTransformContext(xslt_ctxt); xsltCleanupGlobals(); diff --git a/doc/src/sgml/xml2.sgml b/doc/src/sgml/xml2.sgml index 9d302279ab..b0bacd6956 100644 --- a/doc/src/sgml/xml2.sgml +++ b/doc/src/sgml/xml2.sgml @@ -436,14 +436,6 @@ xslt_process(text document, text stylesheet, text paramlist) returns text contain commas! - - Also note that if either the document or stylesheet values do not - begin with a < then they will be treated as URLs and libxslt will - fetch them. It follows that you can use xslt_process as a - means to fetch the contents of URLs — you should be aware of the - security implications of this. - - There is also a two-parameter version of xslt_process which does not pass any parameters to the transformation. -- 2.40.0