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

Reply via email to