Marcus Boerger wrote: > Hello Gregory, > > + 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; > + } > + } > > You missed one part though. C stops execution of a boolean expression on > the first one that decides on the result. So if p[1] is '\0' then p[2] will > never be accessed. So there is no access violation at all.
good point (i.e. duh on my part). attached patch removes that unnecessary paranoia. > Analyzing the check for '..:', took a long time :-) And I like that we > check for this common case without going to the wrapper list. And we do not > need to check for the common case '.' either as you require two chars in > front of the ':', cool! I found a few minor optimizations of this code just now, attached patch should be even better. > > However with the check below: > > + 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; > > Don't we need to check for wrapper being NULL as in: > if (!wrapper || wrapper == &php_plain_files_wrapper) { Probably, I've added that in too. 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 20:01:04 -0000 @@ -447,14 +447,23 @@ char resolved_path[MAXPATHLEN]; char trypath[MAXPATHLEN]; const char *ptr, *end, *p; + char *actual_path; + php_stream_wrapper *wrapper; 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 == ':') && (p[1] == '/') && (p[2] == '/')) { + wrapper = php_stream_locate_url_wrapper(filename, &actual_path, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC); + if (!wrapper || wrapper == &php_plain_files_wrapper) { + if (tsrm_realpath(actual_path, resolved_path TSRMLS_CC)) { + return estrdup(resolved_path); + } + } return NULL; } @@ -473,7 +482,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) && (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 +515,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 +548,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 20:01:04 -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 20:01:05 -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 20:01:06 -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,11 @@ 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 (n == 2 && *path == '.' && path[1] == '.') { + return NULL; + } protocol = path; } else if (n == 5 && strncasecmp(path, "zlib:", 5) == 0) { /* BC with older php scripts and zlib wrapper */ @@ -1754,6 +1758,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 +1770,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 +1833,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 +1883,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