Re: Re: Bug 67072 resolution for 5.4/5.5

Hi! > I think the segfault have to be fixed in spl. This can be done, however can we ensure all classes in PHP and extensions would run properly when unserialized despite explicit prohibition from the class to serialize/unserialize it? Doing O: trick on such class is just not right. Note that crashes is just the start of the problem - what if circumventing prohibited unserialization puts the class into state that allows remote attacker to trick it into doing something it's not supposed to do? Remember the __dtor issue? That one worked on PHP level, this one would work on C level, which would be much worse. > And if we plan to allow newInstanceArgWithoutConstructor() for internal > classes this is mandatory. I'm not sure this is safe either, by the way. But at least here we don't allow remote data to control our class' content and inject any data into it without any controls whatsoever. So here might be a better way out of this. But we need to be very careful with it. > So we can allow "O:.." (perhaps only for empty data used in the > phpunit/doctrine hack, => strlen(*p)<=1) Right now we can not - it leads to remote-triggerable crash in any app that unserializes outside data, and there are lots of these - just search on github for unserialize($_POST or unserialize($_COOKIE. And I'm not sure we can safely allow this in general. I'm sorry that this would make a neat hack unavailable, but I think security of PHP apps is more important than preserving this hack which was never documented and never supposed to work in the first place. I understand that this creates a need that we do not cover of how to mock such objects, and I welcome suggestions - including how to make newInstanceArgWithoutConstructor safe. But currently I do not see how we can leave the unserialize hack in for classes like SplFileObject - unless somebody points me to a way to make it safe. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227