Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:78669
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 108.166.43.115 as permitted sender)
Message-ID: <54591EC3.9080202@sugarcrm.com>
Date: Tue, 04 Nov 2014 10:45:23 -0800
Organization: SugarCRM
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:31.0) Gecko/20100101 Thunderbird/31.2.0
MIME-Version: 1.0
To: Nikita Popov <nikita.ppv@gmail.com>
CC: PHP Internals <internals@lists.php.net>
References: <5457EF60.1020103@sugarcrm.com> <CAF+90c-gRf33Fwqsf00JRLPVuXYjXS2yM4gHYcHUepQ_ynZ9uQ@mail.gmail.com>
In-Reply-To: <CAF+90c-gRf33Fwqsf00JRLPVuXYjXS2yM4gHYcHUepQ_ynZ9uQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] [VOTE] Filtered unserialize()
From: smalyshev@sugarcrm.com (Stas Malyshev)

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/