On Sat, Mar 30, 2013 at 11:05 PM, Anthony Ferrara <ircmax...@gmail.com>wrote:
> Stas, > > Serious question: Why bother trying to clean this up? Why not just > recommend using JSON or some other generic serialization without tieing > into specific objects, and pushing the creation logic into userland (where > it belongs IMHO, at least from a security perspective). At least for any > times where serialized data may come into contact with a user... > > I'm not saying that this isn't a useful addition, I'm just wondering if it > goes down the right path of what we should be recommending to users as the > best practice. I wonder if it'd be better to simply recommend a generic > serialization (JSON, XML, basically something with no class-type > information other than array, object) instead for any use-case where end > users might even remotely be able to tamper with the data. > > That's not to say serialization would be useless, not at all. But more that > serialization (as it stands) would be recommended for server-side only > (which it should be anyway)... > > I just fear this may let some people think that serialized data is OK to > pass to the client. Which is only true with this patch if it's implemented > well and correctly... > > Just my $0.02... Thoughts? > > Anthony > > I agree with Anthony on the note that serialize really only should be used where the serialized data is being stored server-side. I already deal with users that believe the solution to storing compound data in cookies, for example, is OK to do with serialize. Unfortunately, for those that take on this practice and write poorly though-out code they are susceptible to these kinds of security vulnerabilities. Same goes for those who chose to continue using things like register_globals (which luckicly we have removed), but my point is we can't fix poor mentality. We can only educate others on the risks. I think Stas proposes a solution to the problem and I think Anthony proposes a viable alternative. I would say that Anthony has found the shortest distance between the two points (the problem and the solution), however. > > On Sat, Mar 30, 2013 at 10:54 PM, Stas Malyshev <smalys...@sugarcrm.com > >wrote: > > > Hi! > > > > As many probably know, unserialize() has a security issue following from > > the fact that you can create objects with data from unserialize(), and > > these object may have behavior that is invoked automatically - namely > > __destruct - that can result in unintended results. See e.g. > > http://heine.familiedeelstra.com/security/unserialize among others for > > more detailed description. > > > > So I propose a modification to unserialize(): > > https://wiki.php.net/rfc/secure_unserialize > > > > that would make one of the common cases - serializing data to be stored > > on user side or user-accessible side - more secure by avoiding > > instantiating all object (or all objects not belonging to a whitelist) > > and keeping them as incomplete objects instead. > > > > Comments and suggestions welcome, > > -- > > Stanislav Malyshev, Software Architect > > SugarCRM: http://www.sugarcrm.com/ > > (408)454-6900 ext. 227 > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > > > > >
