chregu                                   Tue, 12 Jul 2011 04:58:38 +0000

Revision: http://svn.php.net/viewvc?view=revision&revision=313160

Log:
Added XsltProcessor::setSecurityPrefs($options) and getSecurityPrefs()
to define forbidden operations within XSLT stylesheets, default is not to
enable any write operations from XSLT anymore. Bug #54446

(second iteration of the code for trunk, first commit for 5.4 branch)

Bug: https://bugs.php.net/54446 (To be documented) Arbitrary file creation via 
libxslt 'output' extension
      
Changed paths:
    U   php/php-src/branches/PHP_5_4/NEWS
    U   php/php-src/branches/PHP_5_4/UPGRADING
    U   php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.c
    U   php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.h
    U   php/php-src/branches/PHP_5_4/ext/xsl/xsl_fe.h
    U   php/php-src/branches/PHP_5_4/ext/xsl/xsltprocessor.c
    U   php/php-src/trunk/ext/xsl/xsltprocessor.c

Modified: php/php-src/branches/PHP_5_4/NEWS
===================================================================
--- php/php-src/branches/PHP_5_4/NEWS	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/branches/PHP_5_4/NEWS	2011-07-12 04:58:38 UTC (rev 313160)
@@ -237,6 +237,11 @@
   . Added SplObjectStorage::getHash() hook. (Etienne)
   . Added CallbackFilterIterator and RecursiveCallbackFilterIterator. (Arnaud)

+- Improved XSL extension:
+  . Added XsltProcessor::setSecurityPrefs($options) and getSecurityPrefs() to
+    define forbidden operations within XSLT stylesheets, default is not to
+    enable any write operations from XSLT anymore. Bug #54446
+
 - Improved ZLIB extension:
   . Re-implemented non-file related functionality. (Mike)


Modified: php/php-src/branches/PHP_5_4/UPGRADING
===================================================================
--- php/php-src/branches/PHP_5_4/UPGRADING	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/branches/PHP_5_4/UPGRADING	2011-07-12 04:58:38 UTC (rev 313160)
@@ -176,6 +176,9 @@
   var_export(), and print_r().
 - The raw data parameter in openssl_encrypt()/openssl_decrypt() is now an options
   integer rather than a boolean. A value of true produces the same behaviour.
+- Write operations within XSLT (for example with the extension sax:output) are
+  disabled by default. You can define what is forbidden with the method
+  XsltProcess::setSecurityPrefs($options)

 ===================================
 5. Changes made to existing methods

Modified: php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.c
===================================================================
--- php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.c	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.c	2011-07-12 04:58:38 UTC (rev 313160)
@@ -126,6 +126,7 @@
 	intern->node_list = NULL;
 	intern->doc = NULL;
 	intern->profiling = NULL;
+	intern->securityPrefs = XSL_SECPREF_WRITE_FILE |  XSL_SECPREF_WRITE_NETWORK | XSL_SECPREF_CREATE_DIRECTORY;

 	zend_object_std_init(&intern->std, class_type TSRMLS_CC);
 	object_properties_init(&intern->std, class_type);
@@ -166,6 +167,13 @@
 	REGISTER_LONG_CONSTANT("XSL_CLONE_NEVER",    -1,     CONST_CS | CONST_PERSISTENT);
 	REGISTER_LONG_CONSTANT("XSL_CLONE_ALWAYS",    1,     CONST_CS | CONST_PERSISTENT);

+	REGISTER_LONG_CONSTANT("XSL_SECPREF_NONE",             XSL_SECPREF_NONE,             CONST_CS | CONST_PERSISTENT);
+	REGISTER_LONG_CONSTANT("XSL_SECPREF_READ_FILE",        XSL_SECPREF_READ_FILE,        CONST_CS | CONST_PERSISTENT);
+	REGISTER_LONG_CONSTANT("XSL_SECPREF_WRITE_FILE",       XSL_SECPREF_WRITE_FILE,       CONST_CS | CONST_PERSISTENT);
+	REGISTER_LONG_CONSTANT("XSL_SECPREF_CREATE_DIRECTORY", XSL_SECPREF_CREATE_DIRECTORY, CONST_CS | CONST_PERSISTENT);
+	REGISTER_LONG_CONSTANT("XSL_SECPREF_READ_NETWORK",     XSL_SECPREF_READ_NETWORK,     CONST_CS | CONST_PERSISTENT);
+	REGISTER_LONG_CONSTANT("XSL_SECPREF_WRITE_NETWORK",    XSL_SECPREF_WRITE_NETWORK,    CONST_CS | CONST_PERSISTENT);
+
 	REGISTER_LONG_CONSTANT("LIBXSLT_VERSION",           LIBXSLT_VERSION,            CONST_CS | CONST_PERSISTENT);
 	REGISTER_STRING_CONSTANT("LIBXSLT_DOTTED_VERSION",  LIBXSLT_DOTTED_VERSION,     CONST_CS | CONST_PERSISTENT);


Modified: php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.h
===================================================================
--- php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.h	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/branches/PHP_5_4/ext/xsl/php_xsl.h	2011-07-12 04:58:38 UTC (rev 313160)
@@ -32,6 +32,7 @@
 #include <libxslt/xsltInternals.h>
 #include <libxslt/xsltutils.h>
 #include <libxslt/transform.h>
+#include <libxslt/security.h>
 #if HAVE_XSL_EXSLT
 #include <libexslt/exslt.h>
 #include <libexslt/exsltconfig.h>
@@ -43,6 +44,13 @@
 #include <libxslt/extensions.h>
 #include <libxml/xpathInternals.h>

+#define XSL_SECPREF_NONE 0
+#define XSL_SECPREF_READ_FILE 2
+#define XSL_SECPREF_WRITE_FILE 4
+#define XSL_SECPREF_CREATE_DIRECTORY 8
+#define XSL_SECPREF_READ_NETWORK 16
+#define XSL_SECPREF_WRITE_NETWORK 32
+
 typedef struct _xsl_object {
 	zend_object  std;
 	void *ptr;
@@ -55,6 +63,7 @@
 	HashTable *node_list;
 	php_libxml_node_object *doc;
 	char *profiling;
+	long securityPrefs;
 } xsl_object;

 void php_xsl_set_object(zval *wrapper, void *obj TSRMLS_DC);

Modified: php/php-src/branches/PHP_5_4/ext/xsl/xsl_fe.h
===================================================================
--- php/php-src/branches/PHP_5_4/ext/xsl/xsl_fe.h	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/branches/PHP_5_4/ext/xsl/xsl_fe.h	2011-07-12 04:58:38 UTC (rev 313160)
@@ -34,6 +34,9 @@
 PHP_FUNCTION(xsl_xsltprocessor_has_exslt_support);
 PHP_FUNCTION(xsl_xsltprocessor_register_php_functions);
 PHP_FUNCTION(xsl_xsltprocessor_set_profiling);
+PHP_FUNCTION(xsl_xsltprocessor_set_security_prefs);
+PHP_FUNCTION(xsl_xsltprocessor_get_security_prefs);
+
 #endif

 /*

Modified: php/php-src/branches/PHP_5_4/ext/xsl/xsltprocessor.c
===================================================================
--- php/php-src/branches/PHP_5_4/ext/xsl/xsltprocessor.c	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/branches/PHP_5_4/ext/xsl/xsltprocessor.c	2011-07-12 04:58:38 UTC (rev 313160)
@@ -71,6 +71,13 @@
 ZEND_BEGIN_ARG_INFO_EX(arginfo_xsl_xsltprocessor_set_profiling, 0, 0, 1)
 	ZEND_ARG_INFO(0, filename)
 ZEND_END_ARG_INFO();
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_xsl_xsltprocessor_set_security_prefs, 0, 0, 1)
+	ZEND_ARG_INFO(0, securityPrefs)
+ZEND_END_ARG_INFO();
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_xsl_xsltprocessor_get_security_prefs, 0, 0, 0)
+ZEND_END_ARG_INFO();
 /* }}} */

 /*
@@ -91,6 +98,8 @@
 	PHP_FALIAS(hasExsltSupport, xsl_xsltprocessor_has_exslt_support, arginfo_xsl_xsltprocessor_has_exslt_support)
 	PHP_FALIAS(registerPHPFunctions, xsl_xsltprocessor_register_php_functions, arginfo_xsl_xsltprocessor_register_php_functions)
 	PHP_FALIAS(setProfiling, xsl_xsltprocessor_set_profiling, arginfo_xsl_xsltprocessor_set_profiling)
+	PHP_FALIAS(setSecurityPrefs, xsl_xsltprocessor_set_security_prefs, arginfo_xsl_xsltprocessor_set_security_prefs)
+	PHP_FALIAS(getSecurityPrefs, xsl_xsltprocessor_get_security_prefs, arginfo_xsl_xsltprocessor_get_security_prefs)
 	{NULL, NULL, NULL}
 };

@@ -475,6 +484,8 @@
 	zval *doXInclude, *member;
 	zend_object_handlers *std_hnd;
 	FILE *f;
+	int secPrefsError = 0;
+	xsltSecurityPrefsPtr secPrefs = NULL;

 	node = php_libxml_import_node(docp TSRMLS_CC);

@@ -531,11 +542,54 @@
 	}
 	efree(member);

-	newdocp = xsltApplyStylesheetUser(style, doc, (const char**) params,  NULL, f, ctxt);
+
+	//if securityPrefs is set to NONE, we don't have to do any checks, but otherwise...
+	if (intern->securityPrefs != XSL_SECPREF_NONE) {
+		secPrefs = xsltNewSecurityPrefs();
+		if (intern->securityPrefs & XSL_SECPREF_READ_FILE ) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_READ_FILE, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
+		}
+		if (intern->securityPrefs & XSL_SECPREF_WRITE_FILE ) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_WRITE_FILE, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
+		}
+		if (intern->securityPrefs & XSL_SECPREF_CREATE_DIRECTORY ) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_CREATE_DIRECTORY, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
+		}
+		if (intern->securityPrefs & XSL_SECPREF_READ_NETWORK) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_READ_NETWORK, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
+		}
+		if (intern->securityPrefs & XSL_SECPREF_WRITE_NETWORK) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_WRITE_NETWORK, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
+		}
+
+		if (0 != xsltSetCtxtSecurityPrefs(secPrefs, ctxt)) {
+			secPrefsError = 1;
+		}
+	}
+
+	if (secPrefsError == 1) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties, not doing transformation for security reasons");
+	} else {
+		newdocp = xsltApplyStylesheetUser(style, doc, (const char**) params,  NULL, f, ctxt);
+	}
 	if (f) {
 		fclose(f);
 	}
+
 	xsltFreeTransformContext(ctxt);
+	if (secPrefs) {
+		xsltFreeSecurityPrefs(secPrefs);
+	}

 	if (intern->node_list != NULL) {
 		zend_hash_destroy(intern->node_list);
@@ -857,6 +911,42 @@
 }
 /* }}} end xsl_xsltprocessor_set_profiling */

+/* {{{ proto long xsl_xsltprocessor_set_security_prefs(long securityPrefs) */
+PHP_FUNCTION(xsl_xsltprocessor_set_security_prefs)
+{
+	zval *id;
+	xsl_object *intern;
+	DOM_GET_THIS(id);
+	long securityPrefs, oldSecurityPrefs;
+ 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &securityPrefs) == FAILURE) {
+		return;
+	}
+	intern = (xsl_object *)zend_object_store_get_object(id TSRMLS_CC);
+	oldSecurityPrefs = intern->securityPrefs;
+	intern->securityPrefs = securityPrefs;
+	RETURN_LONG(oldSecurityPrefs);
+}
+/* }}} end xsl_xsltprocessor_set_security_prefs */
+
+/* {{{ proto long xsl_xsltprocessor_get_security_prefs() */
+PHP_FUNCTION(xsl_xsltprocessor_get_security_prefs)
+{
+	zval *id;
+	xsl_object *intern;
+	DOM_GET_THIS(id);
+	long securityPrefs;
+
+	if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "") == SUCCESS) {
+		intern = (xsl_object *)zend_object_store_get_object(id TSRMLS_CC);
+		RETURN_LONG(intern->securityPrefs);
+	} else {
+		WRONG_PARAM_COUNT;
+	}
+}
+/* }}} end xsl_xsltprocessor_get_security_prefs */
+
+
+
 /* {{{ proto bool xsl_xsltprocessor_has_exslt_support();
 */
 PHP_FUNCTION(xsl_xsltprocessor_has_exslt_support)

Modified: php/php-src/trunk/ext/xsl/xsltprocessor.c
===================================================================
--- php/php-src/trunk/ext/xsl/xsltprocessor.c	2011-07-12 04:50:24 UTC (rev 313159)
+++ php/php-src/trunk/ext/xsl/xsltprocessor.c	2011-07-12 04:58:38 UTC (rev 313160)
@@ -484,6 +484,8 @@
 	zval *doXInclude, *member;
 	zend_object_handlers *std_hnd;
 	FILE *f;
+	int secPrefsError = 0;
+	xsltSecurityPrefsPtr secPrefs = NULL;

 	node = php_libxml_import_node(docp TSRMLS_CC);

@@ -540,46 +542,54 @@
 	}
 	efree(member);

-	/* Add security checks */
-	/* XSLT_SECPREF_READ_FILE and XSLT_SECPREF_READ_NETWORK aren't needed */
-
-	xsltSecurityPrefsPtr secPrefs = xsltNewSecurityPrefs();
-
-	if (intern->securityPrefs & XSL_SECPREF_READ_FILE ) {
-		if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_READ_FILE, xsltSecurityForbid)) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties");
+
+	//if securityPrefs is set to NONE, we don't have to do any checks, but otherwise...
+	if (intern->securityPrefs != XSL_SECPREF_NONE) {
+		secPrefs = xsltNewSecurityPrefs();
+		if (intern->securityPrefs & XSL_SECPREF_READ_FILE ) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_READ_FILE, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
 		}
-	}
-	if (intern->securityPrefs & XSL_SECPREF_WRITE_FILE ) {
-		if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_WRITE_FILE, xsltSecurityForbid)) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties");
+		if (intern->securityPrefs & XSL_SECPREF_WRITE_FILE ) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_WRITE_FILE, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
 		}
-	}
-	if (intern->securityPrefs & XSL_SECPREF_CREATE_DIRECTORY ) {
-		if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_CREATE_DIRECTORY, xsltSecurityForbid)) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties");
+		if (intern->securityPrefs & XSL_SECPREF_CREATE_DIRECTORY ) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_CREATE_DIRECTORY, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
 		}
-	}
-	if (intern->securityPrefs & XSL_SECPREF_READ_NETWORK) {
-		if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_READ_NETWORK, xsltSecurityForbid)) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties");
+		if (intern->securityPrefs & XSL_SECPREF_READ_NETWORK) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_READ_NETWORK, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
 		}
-	}
-	if (intern->securityPrefs & XSL_SECPREF_WRITE_NETWORK) {
-		if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_WRITE_NETWORK, xsltSecurityForbid)) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties");
+		if (intern->securityPrefs & XSL_SECPREF_WRITE_NETWORK) {
+			if (0 != xsltSetSecurityPrefs(secPrefs, XSLT_SECPREF_WRITE_NETWORK, xsltSecurityForbid)) {
+				secPrefsError = 1;
+			}
 		}
+
+		if (0 != xsltSetCtxtSecurityPrefs(secPrefs, ctxt)) {
+			secPrefsError = 1;
+		}
 	}
-
-	if (0 != xsltSetCtxtSecurityPrefs(secPrefs, ctxt))
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security handler");
-
-	newdocp = xsltApplyStylesheetUser(style, doc, (const char**) params,  NULL, f, ctxt);
+
+	if (secPrefsError == 1) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Can't set libxslt security properties, not doing transformation for security reasons");
+	} else {
+		newdocp = xsltApplyStylesheetUser(style, doc, (const char**) params,  NULL, f, ctxt);
+	}
 	if (f) {
 		fclose(f);
 	}
+
 	xsltFreeTransformContext(ctxt);
-	xsltFreeSecurityPrefs(secPrefs);
+	if (secPrefs) {
+		xsltFreeSecurityPrefs(secPrefs);
+	}

 	if (intern->node_list != NULL) {
 		zend_hash_destroy(intern->node_list);
@@ -908,15 +918,13 @@
 	xsl_object *intern;
 	DOM_GET_THIS(id);
 	long securityPrefs, oldSecurityPrefs;
-
-	if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "l", &securityPrefs) == SUCCESS) {
-		intern = (xsl_object *)zend_object_store_get_object(id TSRMLS_CC);
-		oldSecurityPrefs = intern->securityPrefs;
-		intern->securityPrefs = securityPrefs;
-		RETURN_LONG(oldSecurityPrefs);
-	} else {
-		WRONG_PARAM_COUNT;
+ 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &securityPrefs) == FAILURE) {
+		return;
 	}
+	intern = (xsl_object *)zend_object_store_get_object(id TSRMLS_CC);
+	oldSecurityPrefs = intern->securityPrefs;
+	intern->securityPrefs = securityPrefs;
+	RETURN_LONG(oldSecurityPrefs);
 }
 /* }}} end xsl_xsltprocessor_set_security_prefs */

-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to