Marcus Boerger wrote:
> Hello Gregory,
>
> Tuesday, March 25, 2008, 8:01:56 PM, you wrote:
>
>> Stanislav Malyshev wrote:
>>>> stream wrapper. Here is an example:
>>>>
>>>> oops.broken://UNC/path
>>> I wonder if .://UNC/path is treated as "."+//UNC/path (and the same
>>> for ..). It should anyway :) However I'm not too worried without
>>> pathes like foo.bar - not likely to have path without any slashes
>>> unless it's . or .., and if you do, you always can say ./foo.bar
>>>
>> That's a great question. In attempting to answer, I think I may have
>> unfortunately found a severe flaw in the patch, allowing reading past
>> the end of the filename and the include_path.
>
>> If we pass a file named "hello:" to php_resolve_path, this code:
>
>> if ((*p == ':') && (p - filename > 1) && (p[1] == '/') && (p[2] == '/')) {
>
>> would look past the end of the filename by 1 (p[1] = '\0', but p[2] is
>> an off by one read). Instead, we should be checking to see if p -
>> filename < filename_length - 2 so we have enough room for the stream
>> wrapper and 1 character (i.e. blah://a, not blah://).
>
> How so?
>
> Just reorder and put (p - filename > 1 first)
<snip>
Hi Marcus,
It looks like I wasn't clear - the problem is not checking characters
before the :, but after. With the existing code, if the string ended
with ":" the code would attempt the check ":" +1 and ":" + 2, which
could result in out-of-bounds read (read after the terminating '\0'.
However, in answer to your other question, we do need stream wrappers
longer than 1 character, as we would otherwise could be unnecessarily
checking drive letters on windows thinking they're stream wrappers.
However, I took your excellent suggestions into account in the attached
patch, which fully works and prevents use of either . or .. as stream
wrapper names.
Otherwise, its identical to wrapper6.patch.txt
One last check by Dmitry before commit and we should be good to go.
Thanks for the catch on the logical problem Stas and the coding
suggestions Marcus - this is the kind of teamwork that I dream of. The
open source gods must be looking down on us kindly.
Greg
Index: main/fopen_wrappers.c
===================================================================
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -0000
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 26 Mar 2008 03:43:28 -0000
@@ -447,14 +447,24 @@
char resolved_path[MAXPATHLEN];
char trypath[MAXPATHLEN];
const char *ptr, *end, *p;
+ char *actual_path;
+ php_stream_wrapper *wrapper;
+ int path_len = strlen(path);
if (!filename) {
return NULL;
}
- /* Don't resolve paths which contain protocol */
+ /* Don't resolve paths which contain protocol (except of file://) */
for (p = filename; isalnum((int)*p) || *p == '+' || *p == '-' || *p ==
'.'; p++);
- if ((*p == ':') && (p - filename > 1) && (p[1] == '/') && (p[2] ==
'/')) {
+ /* checking for enough length after p to ensure we don't read past the
end of filename */
+ if ((*p == ':') && (filename_length - (p - filename) > 2) && (p[1] ==
'/') && (p[2] == '/')) {
+ wrapper = php_stream_locate_url_wrapper(filename, &actual_path,
STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (wrapper == &php_plain_files_wrapper) {
+ if (tsrm_realpath(actual_path, resolved_path
TSRMLS_CC)) {
+ return estrdup(resolved_path);
+ }
+ }
return NULL;
}
@@ -473,7 +483,19 @@
ptr = path;
while (ptr && *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ /* p - ptr > 1 allows us to skip things like "C:\whatever" */
+ if ((*p == ':') && (p - ptr > 1) && (path_len - (p - path) > 2)
&& (p[1] == '/') && (p[2] == '/')) {
+ /* .:// or ..:// is not a stream wrapper */
+ if (p[-1] != '.' || p[-2] != '.' || p - 2 != ptr) {
+ p += 3;
+ is_stream_wrapper = 1;
+ }
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 >= MAXPATHLEN) {
ptr = end + 1;
@@ -494,7 +516,23 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
- if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
+ actual_path = trypath;
+ if (is_stream_wrapper) {
+ wrapper = php_stream_locate_url_wrapper(trypath,
&actual_path, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (!wrapper) {
+ continue;
+ } else if (wrapper != &php_plain_files_wrapper) {
+ if (wrapper->wops->url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper->wops->url_stat(wrapper, trypath, 0, &ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ continue;
+ }
+ }
+ if (tsrm_realpath(actual_path, resolved_path TSRMLS_CC)) {
return estrdup(resolved_path);
}
} /* end provided path */
@@ -511,7 +549,27 @@
exec_fname_length + 1 + filename_length + 1 < MAXPATHLEN) {
memcpy(trypath, exec_fname, exec_fname_length + 1);
memcpy(trypath+exec_fname_length + 1, filename,
filename_length+1);
- if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
+ actual_path = trypath;
+
+ /* Check for stream wrapper */
+ for (p = trypath; isalnum((int)*p) || *p == '+' || *p
== '-' || *p == '.'; p++);
+ if ((*p == ':') && (p - trypath > 1) && (p[1] == '/')
&& (p[2] == '/')) {
+ wrapper =
php_stream_locate_url_wrapper(trypath, &actual_path, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+ if (!wrapper) {
+ return NULL;
+ } else if (wrapper != &php_plain_files_wrapper)
{
+ if (wrapper->wops->url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper->wops->url_stat(wrapper, trypath, 0, &ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ return NULL;
+ }
+ }
+
+ if (tsrm_realpath(actual_path, resolved_path
TSRMLS_CC)) {
return estrdup(resolved_path);
}
}
Index: main/php_streams.h
===================================================================
RCS file: /repository/php-src/main/php_streams.h,v
retrieving revision 1.103.2.1.2.4.2.2
diff -u -r1.103.2.1.2.4.2.2 php_streams.h
--- main/php_streams.h 31 Dec 2007 07:17:17 -0000 1.103.2.1.2.4.2.2
+++ main/php_streams.h 26 Mar 2008 03:43:28 -0000
@@ -511,6 +511,9 @@
/* don't check allow_url_fopen and allow_url_include */
#define STREAM_DISABLE_URL_PROTECTION 0x00002000
+/* assume the path passed in exists and is fully expanded, avoiding syscalls */
+#define STREAM_ASSUME_REALPATH 0x00004000
+
/* Antique - no longer has meaning */
#define IGNORE_URL_WIN 0
Index: main/streams/plain_wrapper.c
===================================================================
RCS file: /repository/php-src/main/streams/plain_wrapper.c,v
retrieving revision 1.52.2.6.2.23.2.5
diff -u -r1.52.2.6.2.23.2.5 plain_wrapper.c
--- main/streams/plain_wrapper.c 31 Dec 2007 07:17:17 -0000
1.52.2.6.2.23.2.5
+++ main/streams/plain_wrapper.c 26 Mar 2008 03:43:29 -0000
@@ -892,9 +892,13 @@
}
return NULL;
}
-
- if ((realpath = expand_filepath(filename, NULL TSRMLS_CC)) == NULL) {
- return NULL;
+
+ if (options & STREAM_ASSUME_REALPATH) {
+ realpath = estrdup(filename);
+ } else {
+ if ((realpath = expand_filepath(filename, NULL TSRMLS_CC)) ==
NULL) {
+ return NULL;
+ }
}
if (persistent) {
Index: main/streams/streams.c
===================================================================
RCS file: /repository/php-src/main/streams/streams.c,v
retrieving revision 1.82.2.6.2.18.2.6
diff -u -r1.82.2.6.2.18.2.6 streams.c
--- main/streams/streams.c 24 Mar 2008 16:28:35 -0000
1.82.2.6.2.18.2.6
+++ main/streams/streams.c 26 Mar 2008 03:43:30 -0000
@@ -1494,7 +1494,7 @@
HashTable *wrapper_hash = (FG(stream_wrappers) ? FG(stream_wrappers) :
&url_stream_wrappers_hash);
php_stream_wrapper **wrapperpp = NULL;
const char *p, *protocol = NULL;
- int n = 0;
+ int n = 0, path_len = strlen(path);
if (path_for_open) {
*path_for_open = (char*)path;
@@ -1508,7 +1508,16 @@
n++;
}
- if ((*p == ':') && (n > 1) && (!strncmp("//", p+1, 2) ||
!memcmp("data", path, 4))) {
+ if ((*p == ':') && (n > 1) && ((path_len - n > 2 && !strncmp("//", p+1,
2)) || (n == 4 && !memcmp("data", path, 4)))) {
+ /* . and .. are invalid stream wrapper names */
+ if (*path == '.') {
+ if (n == 1) {
+ return NULL;
+ }
+ if (path[1] == '.' && n == 2) {
+ return NULL;
+ }
+ }
protocol = path;
} else if (n == 5 && strncasecmp(path, "zlib:", 5) == 0) {
/* BC with older php scripts and zlib wrapper */
@@ -1754,6 +1763,7 @@
php_stream_wrapper *wrapper = NULL;
char *path_to_open;
int persistent = options & STREAM_OPEN_PERSISTENT;
+ char *resolved_path = NULL;
char *copy_of_path = NULL;
@@ -1765,11 +1775,23 @@
return NULL;
}
- path_to_open = path;
+ if (options & USE_PATH) {
+ resolved_path = php_resolve_path(path, strlen(path),
PG(include_path) TSRMLS_CC);
+ if (resolved_path) {
+ path = resolved_path;
+ /* we've found this file, don't re-check include_path
or run realpath */
+ options |= STREAM_ASSUME_REALPATH;
+ options &= ~USE_PATH;
+ }
+ }
+ path_to_open = path;
wrapper = php_stream_locate_url_wrapper(path, &path_to_open, options
TSRMLS_CC);
if (options & STREAM_USE_URL && (!wrapper || !wrapper->is_url)) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "This function may
only be used against URLs");
+ if (resolved_path) {
+ efree(resolved_path);
+ }
return NULL;
}
@@ -1816,12 +1838,18 @@
(options & STREAM_WILL_CAST)
? PHP_STREAM_PREFER_STDIO :
PHP_STREAM_NO_PREFERENCE)) {
case PHP_STREAM_UNCHANGED:
+ if (resolved_path) {
+ efree(resolved_path);
+ }
return stream;
case PHP_STREAM_RELEASED:
if (newstream->orig_path) {
pefree(newstream->orig_path,
persistent);
}
newstream->orig_path = pestrdup(path,
persistent);
+ if (resolved_path) {
+ efree(resolved_path);
+ }
return newstream;
default:
php_stream_close(stream);
@@ -1860,6 +1888,9 @@
pefree(copy_of_path, persistent);
}
#endif
+ if (resolved_path) {
+ efree(resolved_path);
+ }
return stream;
}
/* }}} */
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
