Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:36561 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 50041 invoked from network); 26 Mar 2008 20:01:31 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 26 Mar 2008 20:01:31 -0000 Authentication-Results: pb1.pair.com smtp.mail=greg@chiaraquartet.net; spf=permerror; sender-id=unknown Authentication-Results: pb1.pair.com header.from=greg@chiaraquartet.net; sender-id=unknown Received-SPF: error (pb1.pair.com: domain chiaraquartet.net from 38.99.98.18 cause and error) X-PHP-List-Original-Sender: greg@chiaraquartet.net X-Host-Fingerprint: 38.99.98.18 beast.bluga.net Linux 2.6 Received: from [38.99.98.18] ([38.99.98.18:49202] helo=mail.bluga.net) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id F0/2A-47041-A9BAAE74 for ; Wed, 26 Mar 2008 15:01:31 -0500 Received: from mail.bluga.net (localhost.localdomain [127.0.0.1]) by mail.bluga.net (Postfix) with ESMTP id AD214C0FDB7; Wed, 26 Mar 2008 13:01:28 -0700 (MST) Received: from [192.168.0.106] (CPE-76-84-4-101.neb.res.rr.com [76.84.4.101]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.bluga.net (Postfix) with ESMTP id A89C8C0FDAE; Wed, 26 Mar 2008 13:01:27 -0700 (MST) Message-ID: <47EAABAB.4040604@chiaraquartet.net> Date: Wed, 26 Mar 2008 15:01:47 -0500 User-Agent: Thunderbird 2.0.0.12 (X11/20080227) MIME-Version: 1.0 To: Marcus Boerger CC: internals@lists.php.net, Dmitry Stogov 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> <99395441.20080326122856@marcus-boerger.de> In-Reply-To: <99395441.20080326122856@marcus-boerger.de> Content-Type: multipart/mixed; boundary="------------090305020402030101010009" X-Virus-Scanned: ClamAV using ClamSMTP Subject: Re: [PHP-DEV] REMINDER - stream wrappers in include_path From: greg@chiaraquartet.net (Gregory Beaver) --------------090305020402030101010009 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 --------------090305020402030101010009 Content-Type: text/plain; name="wrapper9.patch.txt" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="wrapper9.patch.txt" 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; } /* }}} */ --------------090305020402030101010009--