Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:78673
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 209.85.214.181 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <54591EC3.9080202@sugarcrm.com>
References: <5457EF60.1020103@sugarcrm.com>
	<CAF+90c-gRf33Fwqsf00JRLPVuXYjXS2yM4gHYcHUepQ_ynZ9uQ@mail.gmail.com>
	<54591EC3.9080202@sugarcrm.com>
Date: Tue, 4 Nov 2014 23:48:22 +0400
Message-ID: <CA+9eiLsD5-zqSHR1u4b=jb7sEDR7COTKNxpzOBPXJfTqnrqspQ@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: Nikita Popov <nikita.ppv@gmail.com>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c30fbce85f8205070dbe3f
Subject: Re: [PHP-DEV] [RFC] [VOTE] Filtered unserialize()
From: dmitry@zend.com (Dmitry Stogov)

--001a11c30fbce85f8205070dbe3f
Content-Type: text/plain; charset=UTF-8

I agree with Nikita.
Adding an extra argument for one particular security related case looks
weird.
Will we add more arguments for others?

I would also prefer options array and [] instead of 'false'.

unserialize($string, array('allowed_classes'=>[]));

It's also more readable :)

Thanks. Dmitry.

On Tue, Nov 4, 2014 at 9:45 PM, Stas Malyshev <smalyshev@sugarcrm.com>
wrote:

> Hi!
>
> > I'm -1 on this RFC, because I think this only further encourages
> > ill-advised usages of unserialize() on user-provided strings. I don't
>
> I guess that's where we disagree. I think that security is a layered
> approach (see more here:
> http://php100.wordpress.com/2014/11/03/unserialize-and-being-practical/).
> Some
> people think that if somebody deviates from the best practice, however
> good are the reasons, there should be no support whatsoever in securing
> the alternative approach since it "encourages" departing from best
> practices. I think this approach is wrong.
>
> > format allows you to create references. I'd imagine that you can easily
> > use this to cause a DOS condition if the code processing the unserialize
> > output uses any kind of recursion.
>
> I'm not sure what you mean here. I'm not aware of any way to cause any
> recursion or DoS when parsing serialized data, as for user-side data
> processing, of course I do not attempt to cure the world and solve the
> halting problem, I only give you a tool to filter data. If you store
> recursive data structures in your data and process it, this RFC does not
> provide recursion protection, we can not do everything in one RFC :)
>
> > Furthermore I dislike some details of the particular implementation: The
> > ability to use false as a synonym for [] seems unnecessary. Directly
>
> I could make it produce an error on false, but I don't see what use case
> it would help. If you have such use case, please describe it.
>
> > using an extra argument will be inconvenient for future additions, e.g.
> > if you really wanted to more this secure, you'd probably also want to
> > have options to disable references and to limit the cumulative number of
> > array elements (hashdos). I'd prefer using an options array for this.
>
> I sill have hope named parameters happen some time, which would
> eliminate the need option arrays. Since right now we have only one
> option, I think option array is redundant here.
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--001a11c30fbce85f8205070dbe3f--