Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:78672
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.46 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 20:26:37 +0100
Message-ID: <CAF+90c8p-NTw9PEmmNK5EBHSOGJWEAunddk-C92E2-Of2V64QQ@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=f46d0442889625d35905070d7137
Subject: Re: [PHP-DEV] [RFC] [VOTE] Filtered unserialize()
From: nikita.ppv@gmail.com (Nikita Popov)

--f46d0442889625d35905070d7137
Content-Type: text/plain; charset=UTF-8

On Tue, Nov 4, 2014 at 7: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.
>

To clarify: I don't think it makes sense to add an additional security
option, if we cannot say that unserialize() is to our knowledge *fully*
secure if this option is used. Adding this makes sense if you can say "if
you use this option, you can safely use unserialize() on user-provided
input, nothing bad can happen, ignore anything we told you previously". I
don't think that adding an allowed classes list quite gets us to that
point. The two issues I see are the ability to create references and to
exploit hashdos (the latter also applies to json). Maybe there are others,
this would need closer review.


> > 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.
>

Just looking at your implementation again, it looks like "false" is not a
special value and you actually accept anything, regardless of type. So if I
pass the string "foo" as the second parameter, it will simply do nothing.
Silently discarding values for security features sounds dubious to me.

Nikita

--f46d0442889625d35905070d7137--