Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:50240
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain lerdorf.com from 74.125.83.170 cause and error)
Message-ID: <4CE24365.6040509@lerdorf.com>
Date: Tue, 16 Nov 2010 00:40:05 -0800
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.12) Gecko/20101027 Lightning/1.0b2 Thunderbird/3.1.6
MIME-Version: 1.0
To: Dmitry Stogov <dmitry@zend.com>
CC: Stas Malyshev <smalyshev@sugarcrm.com>, Andi Gutmans <andi@zend.com>, 
 internals <internals@lists.php.net>
References: <4CE03E41.9030805@lerdorf.com> <8757232E56758B42B2EE4F9D2CA019C9086B69@US-EX2.zend.net> <4CE10E8E.3070901@lerdorf.com> <4CE206C0.5070701@sugarcrm.com> <8757232E56758B42B2EE4F9D2CA019C908A06A@US-EX2.zend.net> <4CE215A3.30606@lerdorf.com> <4CE220CB.5090005@sugarcrm.com> <4CE22181.8090904@lerdorf.com> <4CE23BFE.30403@zend.com>
In-Reply-To: <4CE23BFE.30403@zend.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Adding path_len to all stream functions in trunk
From: rasmus@lerdorf.com (Rasmus Lerdorf)

Yes, you could say that users should just know to filter, and if we were
able to pass the binary string through and had binary-safe filesystem
calls, I could agree that we wouldn't need to do anything, but the
current default of truncating at the null byte is dangerous.

Compare these:

% echo '/etc/passwd\0.txt'
/etc/passwd.txt
% cat '/etc/passwd\0.txt'
cat: /etc/passwd\0.txt: No such file or directory

The same thing from PHP:

echo "/etc/passwd\0.txt";
readfile("/etc/passwd\0.txt");

In both cases the echo prints out a binary-safe string with a null byte
before the ".txt" part.  However, unlike UNIX cat, PHP will show
/etc/passwd there.  The cat command will interpret its argument
differently from the echo command because cat knows its argument is a
filesystem path and a literal null makes no sense there.

So, the argument is that PHP should be as smart as cat here.

-Rasmus

On 11/16/10 12:08 AM, Dmitry Stogov wrote:
> hi,
> 
> don't we have ext/filter that should check all the dangerous input
> strings? It would be useless to perform additional checks for constant
> stings known at compile time (e.g. on include "foo.php")
> 
> Thanks. Dmitry.
> 
> Rasmus Lerdorf wrote:
>> On 11/15/10 10:12 PM, Stas Malyshev wrote:
>>> Hi!
>>>
>>>> Well, it changes the signature of that function, so while we don't
>>>> break
>>>> backward binary compatibility, we break forward compatibility within
>>>> the
>>>> 5.3 branch.  As in, if I change my extension to use this new NoNull
>>>> string flag, it will no longer work on<5.3.3 whereas if I do the
>>>> if(strlen(filename) != filename_len) check, this will still work in all
>>>> 5.3 releases.
>>> So if you have such extension, and you need to have it compatible with
>>> previous versions (e.g. PECL one), use the check. That doesn't prevent
>>> us from having the flag in the core code and thus keeping it cleaner.
>>
>> It still worries me a bit.  Distros love to separate core extensions
>> into separate packages and if you update one of those without updating
>> the core package, it will break.  Hopefully they have hard dependencies
>> so you can't install php-curl-5.3.4 on php-5.3.3, for example.
>>
>> -Rasmus
>>