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)
Here also note that this checks for two chars not just one. One woudl be
simplier as it could be done with: (p > filename)
The remainder is: p[0] == ':' && p[1] == '/' && p[2] == '/'
Now thanks to C rules p[1] is never accessed if p[0] is not ':' which for
instance is true for '\0'. So unless p can be of or filename is not zero
terminated we are fine.
Now p is set to filename initially and checks for some chars. So it can
only whatever but must be equal to filename or greater filename.
So in the end you can do:
if (*p == ':' && ++p > filename && *p == '/' && p[1] == '/')
Only a bit harder to read. The only question that still remains to me is
why you require two chars in front of the ':'. Also to deal with '.' and
'..' you can simply add a check for p[-1] != '.' or p[-2] != '.' after the
++p.
> To get around the problem Stas raises, we need to disallow "." or ".."
> as stream wrapper names in php_stream_locate_url_wrapper and check
> for them explicitly in php_resolve_path.
> The attached patch is identical to Dmitry's wrapper6.patch.txt and fixes
> these two issues.
> Note that the check for "." and ".." is only needed when scanning
> include_path, and the part at the end that checks dirname(__FILE__) does
> not need it because there is no way __FILE__ could be
> .://path/to/something or ..:/path/to/something if . and .. are
> disallowed as stream wrapper names.
> Unfortunately, I don't have quite enough time to get the attached patch
> working, but I'm including it for the smarties to figure out how to
> handle, I've put /* XXX FIXME */ where things need work.
> Thanks,
> Greg
marcus
Best regards,
Marcus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
