Re: REMINDER - stream wrappers in include_path

From: Date: Wed, 26 Mar 2008 03:46:45 +0000
Subject: Re: REMINDER - stream wrappers in include_path
References: 1 2 3 4 5 6 7 8 9  Groups: php.internals 
Request: Send a blank email to internals+get-36525@lists.php.net to get a copy of this message
Marcus Boerger wrote: > Hello Gregory, > > Tuesday, March 25, 2008, 8:01:56 PM, you wrote: > >> Stanislav Malyshev wrote: >>>> stream wrapper. Here is an example: >>>> >>>> oops.broken://UNC/path >>> I wonder if .://UNC/path is treated as "."+//UNC/path (and the same >>> for ..). It should anyway :) However I'm not too worried without >>> pathes like foo.bar - not likely to have path without any slashes >>> unless it's . or .., and if you do, you always can say ./foo.bar >>> >> That's a great question. In attempting to answer, I think I may have >> unfortunately found a severe flaw in the patch, allowing reading past >> the end of the filename and the include_path. > >> If we pass a file named "hello:" to php_resolve_path, this code: > >> if ((*p == ':') && (p - filename > 1) && (p[1] == >> '/') && (p[2] == '/')) { > >> would look past the end of the filename by 1 (p[1] = '\0', but p[2] is >> an off by one read). Instead, we should be checking to see if p - >> filename < filename_length - 2 so we have enough room for the stream >> wrapper and 1 character (i.e. blah://a, not blah://). > > How so? > > Just reorder and put (p - filename > 1 first) <snip> Hi Marcus, It looks like I wasn't clear - the problem is not checking characters before the :, but after. With the existing code, if the string ended with ":" the code would attempt the check ":" +1 and ":" + 2, which could result in out-of-bounds read (read after the terminating '\0'. However, in answer to your other question, we do need stream wrappers longer than 1 character, as we would otherwise could be unnecessarily checking drive letters on windows thinking they're stream wrappers. However, I took your excellent suggestions into account in the attached patch, which fully works and prevents use of either . or .. as stream wrapper names. Otherwise, its identical to wrapper6.patch.txt One last check by Dmitry before commit and we should be good to go. Thanks for the catch on the logical problem Stas and the coding suggestions Marcus - this is the kind of teamwork that I dream of. The open source gods must be looking down on us kindly. Greg

« previous php.internals (#36525) next »