Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:66873
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain mrclay.org from 50.22.11.19 cause and error)
Message-ID: <5158847D.4080806@mrclay.org>
Date: Sun, 31 Mar 2013 14:46:21 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/20130307 Thunderbird/17.0.4
MIME-Version: 1.0
To: PHP Internals <internals@lists.php.net>
References: <5157A55A.1070507@sugarcrm.com> <CAAyV7nGtz94NaFdeQ4KL9X3_LX-dc=zbt35o=qoqd1zWGTFgaA@mail.gmail.com> <CAL+t6cF_pzrQtNSYMjCmxhm67QtbfhpEUiWx4xGWxAJpafrwkQ@mail.gmail.com> <5157AD1D.3020606@sugarcrm.com>
In-Reply-To: <5157AD1D.3020606@sugarcrm.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] more secure unserialize()
From: steve@mrclay.org (Steve Clay)

I think this RFC would worsen the problem of misusing the serialize round-trip.

Even if we make the docs clearer, we'd still be sending the message that there's a new 
"safer" unserialize and some would certainly use that new feature to be more lax about 
guarding serialized structures.

It also sets up a situation where altering either the whitelist or one of the classes in 
the whitelist could open a vulnerability that's not obvious.

I'm also not convinced that this feature would spur developers to fix insecure code.

But setting my arguments against, if the goal is to make unserialize() secure, then it 
should behave like a tripwire: fail loudly if a non-whitelisted class object is found. I 
think returning partially-usable values would gives devs more rope to hang themselves with.

Re the 2nd arg, I'd make only two cases:
* null is given: default behavior
* non-null given: cast to array and that's the class whitelist.


If the overall goal is to make the serialize/unserialize round-trip tamper-proof, we could 
build HMAC right into the API: add secret key args to both functions. No doubt the Suoshin 
patch already uses HMAC during encryption of the session data.

Steve Clay
-- 
http://www.mrclay.org/