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.

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!

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) {

marcus

Wednesday, March 26, 2008, 4:46:45 AM, you wrote:

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;
  

}
/* }}} */

Best regards,
Marcus