Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:36533 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 59248 invoked from network); 26 Mar 2008 11:29:00 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 26 Mar 2008 11:29:00 -0000 Authentication-Results: pb1.pair.com smtp.mail=helly@php.net; spf=unknown; sender-id=unknown Authentication-Results: pb1.pair.com header.from=helly@php.net; sender-id=unknown Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.94.56 as permitted sender) X-PHP-List-Original-Sender: helly@php.net X-Host-Fingerprint: 85.214.94.56 aixcept.net Linux 2.6 Received: from [85.214.94.56] ([85.214.94.56:55827] helo=h1149922.serverkompetenz.net) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id C3/72-47041-B733AE74 for ; Wed, 26 Mar 2008 06:29:00 -0500 Received: from dhcp-172-28-202-230.zrh.corp.google.com (unknown [193.142.125.1]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by h1149922.serverkompetenz.net (Postfix) with ESMTP id ED26311DB6A; Wed, 26 Mar 2008 12:28:56 +0100 (CET) Date: Wed, 26 Mar 2008 12:28:56 +0100 Reply-To: Marcus Boerger X-Priority: 3 (Normal) Message-ID: <99395441.20080326122856@marcus-boerger.de> To: Gregory Beaver CC: internals@lists.php.net In-Reply-To: <47E9C725.4060308@chiaraquartet.net> References: <47E2F8FA.20107@chiaraquartet.net> <47E7FEFD.8080504@chiaraquartet.net> <47E800F1.5090101@zend.com> <200803251613.20245.et@php.net> <47E91898.5050503@zend.com> <47E92D3B.7010500@chiaraquartet.net> <47E930C0.6000201@zend.com> <47E94C24.2030400@chiaraquartet.net> <1891676371.20080325201729@marcus-boerger.de> <47E9C725.4060308@chiaraquartet.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Subject: Re: [PHP-DEV] REMINDER - stream wrappers in include_path From: helly@php.net (Marcus Boerger) 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