Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:50243
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain lerdorf.com from 209.85.212.170 cause and error)
Message-ID: <4CE25586.1070309@lerdorf.com>
Date: Tue, 16 Nov 2010 01:57:26 -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: Derick Rethans <derick@php.net>
CC: Andi Gutmans <andi@zend.com>, internals <internals@lists.php.net>
References: <4CE03E41.9030805@lerdorf.com> <8757232E56758B42B2EE4F9D2CA019C9086B69@US-EX2.zend.net> <4CE10E8E.3070901@lerdorf.com> <alpine.DEB.2.00.1011160940390.26633@kossu.derickrethans.nl>
In-Reply-To: <alpine.DEB.2.00.1011160940390.26633@kossu.derickrethans.nl>
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)

On 11/16/10 1:44 AM, Derick Rethans wrote:
> On Mon, 15 Nov 2010, Rasmus Lerdorf wrote:
> 
>> Ok, I went through all the 5.3 code.  This should fix the null poisoning
>> problems in 5.3 without breaking binary compatibility:
>>
>> http://progphp.com/nullpatch.txt
>>
>> There are quite a few places where we can't solve it centrally, so
>> perhaps we need to take the same approach in trunk.
> 
> I've had a look at that patch, and it looks a little bit like a kludge.
> I'm saying that because it's quite easy to miss a specific case where a 
> line like:
> 
> +	if (strlen(filename) != filename_len) {
> +		RETURN_FALSE;
> +	}
> 
> should/could be added. I prefer a fix that solves this properly, and 
> that requires breaking BC as I understood it. Perhaps we could just take 
> care of this in trunk only? Or at last, add a macro for doing the check 
> from above?

Yes, I agree it is a bit of a kludge, but no matter how we do it, it
will be easy to miss a case as there simply isn't a central place to do
this check that will catch all cases.  We can make the check prettier by
adding this non-null params type, or a pretty macro, but that doesn't
alleviate the need to go through and find all the places where we need
to do the check.

One thing that makes it a bit easier is that in many instances, but not
all, the check is near an open_basedir check.

-Rasmus