Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:30142
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 63.205.162.114 as permitted sender)
Message-ID: <466D95CB.7010906@zend.com>
Date: Mon, 11 Jun 2007 11:34:51 -0700
Organization: Zend Technologies
User-Agent: Thunderbird 2.0.0.0 (Windows/20070326)
MIME-Version: 1.0
To: php-dev <internals@lists.php.net>
References: <464DCB8C.90803@chiaraquartet.net> <464DEF23.3080503@zend.com> <464DF139.6090405@zend.com> <464E1AA8.9050600@php.net> <465CC25E.9080309@zend.com>
In-Reply-To: <465CC25E.9080309@zend.com>
Content-Type: multipart/mixed;
 boundary="------------000009060808020005070006"
Subject: Re: [PATCH] potential solution to user streams + allow_url_include=off
From: stas@zend.com (Stanislav Malyshev)

--------------000009060808020005070006
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Attached is second version of the patch implementing the security model 
below. The difference to previous version is that the optional argument 
for stream_wrapper_register is now an integer flag, which allows to 
extend it in the future.

Any comments/objections?

>>>> I think the problem could be solved this way:
>>>> 1. By default, allow_remote_inclue=0, allow_remote_fopen=1
>>>> 2. Stream can be of three types - remote, local and user/local.
>>>> 3. User streams can be declared when registered as either remote or
>>>> user/local, remote being the default.
>>>> 4. When operation on user/local stream is run, allow_remote_fopen is
>>>> disabled if allow_remote_include was disabled.
> 

-- 
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/

--------------000009060808020005070006
Content-Type: text/plain;
 name="streams2.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="streams2.diff"

Index: main/streams/userspace.c
===================================================================
RCS file: /repository/php-src/main/streams/userspace.c,v
retrieving revision 1.42
diff -u -r1.42 userspace.c
--- main/streams/userspace.c	15 May 2007 13:01:47 -0000	1.42
+++ main/streams/userspace.c	11 Jun 2007 18:33:53 -0000
@@ -81,6 +81,7 @@
 	REGISTER_LONG_CONSTANT("STREAM_URL_STAT_QUIET", 	PHP_STREAM_URL_STAT_QUIET,		CONST_CS|CONST_PERSISTENT);
 	REGISTER_LONG_CONSTANT("STREAM_MKDIR_RECURSIVE",	PHP_STREAM_MKDIR_RECURSIVE,		CONST_CS|CONST_PERSISTENT);
 
+	REGISTER_LONG_CONSTANT("STREAM_IS_LOCAL",	PHP_STREAM_IS_LOCAL,		CONST_CS|CONST_PERSISTENT);
 	return SUCCESS;
 }
 
@@ -215,6 +216,7 @@
 	int call_result;
 	php_stream *stream = NULL;
 	zval *zcontext = NULL;
+	char *old_allow_value = NULL;
 
 	/* Try to catch bad usage without preventing flexibility */
 	if (FG(user_stream_current_filename) != NULL && strcmp(filename, FG(user_stream_current_filename)) == 0) {
@@ -223,6 +225,28 @@
 	}
 	FG(user_stream_current_filename) = filename;
 	
+	/* if the user stream was registered as local and we are in include context,
+		we add allow_url_include restrictions to allow_url_fopen ones */
+	/* we need only is_url == 0 here since if is_url == 1 and remote wrappers
+		were restricted we wouldn't get here */
+	if(uwrap->wrapper.is_url == 0 && 
+		(options & STREAM_OPEN_FOR_INCLUDE) && 
+		(PG(allow_url_include_list) == NULL || strlen(PG(allow_url_include_list)) !=1 || PG(allow_url_include_list)[0] != '*')) {
+		/* we are in include and allow_url_include is restrictive */
+		char *include_value = zend_ini_string("allow_url_include", sizeof("allow_url_include"), 0);
+		old_allow_value = zend_ini_string("allow_url_fopen", sizeof("allow_url_fopen"), 0);
+		if(old_allow_value) {
+			old_allow_value = estrdup(old_allow_value);
+		}
+		if (zend_alter_ini_entry("allow_url_fopen", sizeof("allow_url_fopen"), 
+						include_value, strlen(include_value),
+						PHP_INI_USER, PHP_INI_STAGE_RUNTIME) == FAILURE) {
+			php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "allow_url_fopen is more restrictive than allow_url_include");
+			efree(old_allow_value);
+			return NULL;
+		}
+	}
+
 	us = emalloc(sizeof(*us));
 	us->wrapper = uwrap;	
 
@@ -258,6 +282,15 @@
 			FREE_ZVAL(us->object);
 			efree(us);
 			FG(user_stream_current_filename) = NULL;
+			if(old_allow_value) {
+				zend_alter_ini_entry("allow_url_fopen", sizeof("allow_url_fopen"), 
+					old_allow_value, strlen(old_allow_value),
+					PHP_INI_USER, PHP_INI_STAGE_STARTUP);
+				/* hack here - using PHP_INI_STAGE_STARTUP since we need the value back 
+				but we can't just use restore_ini_entry since it will restore 
+				default value, not current value */
+				efree(old_allow_value);
+			}
 			return NULL;
 		} else {
 			if (retval_ptr) {
@@ -338,7 +371,16 @@
 	zval_ptr_dtor(&zfilename);
 
 	FG(user_stream_current_filename) = NULL;
-		
+
+	if(old_allow_value) {
+		zend_alter_ini_entry("allow_url_fopen", sizeof("allow_url_fopen"), 
+								old_allow_value, strlen(old_allow_value),
+								PHP_INI_USER, PHP_INI_STAGE_STARTUP);
+		/* hack here - using PHP_INI_STAGE_STARTUP since we need the value back 
+			but we can't just use restore_ini_entry since it will restore 
+			default value, not current value */
+		efree(old_allow_value);
+	}
 	return stream;
 }
 
@@ -351,6 +393,7 @@
 	zval **args[2];	
 	int call_result;
 	php_stream *stream = NULL;
+	char *old_allow_value = NULL;
 
 	/* Try to catch bad usage without preventing flexibility */
 	if (FG(user_stream_current_filename) != NULL && strcmp(filename, FG(user_stream_current_filename)) == 0) {
@@ -359,6 +402,28 @@
 	}
 	FG(user_stream_current_filename) = filename;
 	
+	/* if the user stream was registered as local and we are in include context,
+		we add allow_url_include restrictions to allow_url_fopen ones */
+	/* we need only is_url == 0 here since if is_url == 1 and remote wrappers
+		were restricted we wouldn't get here */
+	if(uwrap->wrapper.is_url == 0 && 
+		(options & STREAM_OPEN_FOR_INCLUDE) && 
+		(PG(allow_url_include_list) == NULL || strlen(PG(allow_url_include_list)) !=1 || PG(allow_url_include_list)[0] != '*')) {
+		/* we are in include and allow_url_include is restrictive */
+		char *include_value = zend_ini_string("allow_url_include", sizeof("allow_url_include"), 0);
+		old_allow_value = zend_ini_string("allow_url_fopen", sizeof("allow_url_fopen"), 0);
+		if(old_allow_value) {
+			old_allow_value = estrdup(old_allow_value);
+		}
+		if (zend_alter_ini_entry("allow_url_fopen", sizeof("allow_url_fopen"), 
+							include_value, strlen(include_value),
+							PHP_INI_USER, PHP_INI_STAGE_RUNTIME) == FAILURE) {
+			php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "allow_url_fopen is more restrictive than allow_url_include");
+			efree(old_allow_value);
+			return NULL;
+		}
+	}
+
 	us = emalloc(sizeof(*us));
 	us->wrapper = uwrap;	
 
@@ -424,11 +489,20 @@
 
 	FG(user_stream_current_filename) = NULL;
 		
+	if(old_allow_value) {
+		zend_alter_ini_entry("allow_url_fopen", sizeof("allow_url_fopen"), 
+			old_allow_value, strlen(old_allow_value),
+			PHP_INI_USER, PHP_INI_STAGE_STARTUP);
+		/* hack here - using PHP_INI_STAGE_STARTUP since we need the value back 
+		but we can't just use restore_ini_entry since it will restore 
+		default value, not current value */
+		efree(old_allow_value);
+	}
 	return stream;
 }
 
 
-/* {{{ proto bool stream_wrapper_register(string protocol, string classname)
+/* {{{ proto bool stream_wrapper_register(string protocol, string classname[, int flags])
    Registers a custom URL protocol handler class */
 PHP_FUNCTION(stream_wrapper_register)
 {
@@ -436,8 +510,9 @@
 	int protocol_len, classname_len;
 	struct php_user_stream_wrapper * uwrap;
 	int rsrc_id;
+	long flags = 0;
 	
-	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss", &protocol, &protocol_len, &classname, &classname_len) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss|l", &protocol, &protocol_len, &classname, &classname_len, &flags) == FAILURE) {
 		RETURN_FALSE;
 	}
 	
@@ -446,6 +521,7 @@
 	uwrap->classname = estrndup(classname, classname_len);
 	uwrap->wrapper.wops = &user_stream_wops;
 	uwrap->wrapper.abstract = uwrap;
+	uwrap->wrapper.is_url = ((flags & PHP_STREAM_IS_LOCAL) != 0);
 
 	rsrc_id = ZEND_REGISTER_RESOURCE(NULL, uwrap, le_protocols);
 
Index: ext/standard/basic_functions.c
===================================================================
RCS file: /repository/php-src/ext/standard/basic_functions.c,v
retrieving revision 1.859
diff -u -r1.859 basic_functions.c
--- ext/standard/basic_functions.c	9 Jun 2007 11:42:55 -0000	1.859
+++ ext/standard/basic_functions.c	11 Jun 2007 18:33:53 -0000
@@ -205,9 +205,10 @@
 /* }}} */
 /* {{{ main/streams/userspace.c */
 static
-ZEND_BEGIN_ARG_INFO(arginfo_stream_wrapper_register, 0)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_stream_wrapper_register, 0, 0, 2)
 	ZEND_ARG_INFO(0, protocol)
 	ZEND_ARG_INFO(0, classname)
+	ZEND_ARG_INFO(0, flags)
 ZEND_END_ARG_INFO()
 
 static
@@ -2347,6 +2348,11 @@
 ZEND_END_ARG_INFO()
 
 static
+ZEND_BEGIN_ARG_INFO(arginfo_stream_is_local, 0)
+	ZEND_ARG_INFO(0, stream)
+ZEND_END_ARG_INFO()
+
+static
 ZEND_BEGIN_ARG_INFO_EX(arginfo_stream_select, 0, 0, 4)
 	ZEND_ARG_INFO(1, read_streams) /* ARRAY_INFO(1, read_streams, 1) */
 	ZEND_ARG_INFO(1, write_streams) /* ARRAY_INFO(1, write_streams, 1) */
@@ -3596,6 +3602,7 @@
 	PHP_FE(stream_wrapper_restore,											arginfo_stream_wrapper_restore)
 	PHP_FE(stream_get_wrappers,												arginfo_stream_get_wrappers)
 	PHP_FE(stream_get_transports,											arginfo_stream_get_transports)
+	PHP_FE(stream_is_local,												arginfo_stream_is_local)
 	PHP_FE(get_headers,														arginfo_get_headers)
 
 #if HAVE_SYS_TIME_H || defined(PHP_WIN32)
Index: ext/standard/streamsfuncs.c
===================================================================
RCS file: /repository/php-src/ext/standard/streamsfuncs.c,v
retrieving revision 1.103
diff -u -r1.103 streamsfuncs.c
--- ext/standard/streamsfuncs.c	12 Apr 2007 13:15:17 -0000	1.103
+++ ext/standard/streamsfuncs.c	11 Jun 2007 18:33:53 -0000
@@ -1647,6 +1647,37 @@
 }
 /* }}} */
 
+/* {{{ proto bool stream_is_local(resource stream|string url) U
+*/
+PHP_FUNCTION(stream_is_local)
+{
+	zval *zstream;
+	php_stream *stream = NULL;
+	php_stream_wrapper *wrapper = NULL;
+
+	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z", &zstream) == FAILURE) {
+		RETURN_FALSE;
+	}
+
+	if(Z_TYPE_P(zstream) == IS_RESOURCE) {
+		php_stream_from_zval(stream, &zstream);
+		if(stream == NULL) {
+			RETURN_FALSE;
+		}
+		wrapper = stream->wrapper;
+	} else {
+		convert_to_string_ex(&zstream);
+		wrapper = php_stream_locate_url_wrapper(Z_STRVAL_P(zstream), NULL, STREAM_LOCATE_WRAPPERS_ONLY TSRMLS_CC);
+	}
+
+	if(!wrapper) {
+		RETURN_FALSE;
+	}
+
+	RETURN_BOOL(wrapper->is_url==0);
+}
+/* }}} */
+
 #ifdef HAVE_SHUTDOWN
 /* {{{ proto int stream_socket_shutdown(resource stream, int how) U
 	causes all or part of a full-duplex connection on the socket associated
Index: ext/standard/streamsfuncs.h
===================================================================
RCS file: /repository/php-src/ext/standard/streamsfuncs.h,v
retrieving revision 1.19
diff -u -r1.19 streamsfuncs.h
--- ext/standard/streamsfuncs.h	1 Jan 2007 09:29:32 -0000	1.19
+++ ext/standard/streamsfuncs.h	11 Jun 2007 18:33:53 -0000
@@ -57,6 +57,7 @@
 PHP_FUNCTION(stream_socket_shutdown);
 PHP_FUNCTION(stream_socket_pair);
 PHP_FUNCTION(stream_resolve_include_path);
+PHP_FUNCTION(stream_is_local);
 
 /*
  * Local variables:

--------------000009060808020005070006--