Re: Re: Bug 67072 resolution for 5.4/5.5

On 23/06/14 18:41, Stas Malyshev wrote: >> <?php >> > class FooFile extends SplFileInfo { >> > } >> > $str = 'O:7:"FooFile":0:{}'; >> > var_dump(unserialize($str)); >> > ?> > I'm afraid here we can't do much - SplFileInfo is one of the classes > that it is unsafe to instantiate this way. It's what original 67072 was > about and I don't think it's safe to leave it this way, since at best it > can crash any code that uses unserialize() on external data, at worst > you got RCE. As I see it, the main problem here is trying to cheat when creating a restored process? Rather than correctly calling the class and instantiating it via the normal process, some third party process creates a fake saved version of the object and tries to restart it bypassing the proper construction process? That many objects are not designed to be constructed in this way is the problem, and PHP5.4 and 5.5 are not the platform to be making changes to that process? Personally I think that bypassing the 'constructor' can be viewed two ways. Originally PHP did not even have the need for it, as it has been a more recent addition, so that it has unforeseen side effects is to be expected? NOW changing things back so that it is not necessary seems to be counter intuitive? There were good reasons for insisting on the constructor stage, and seeding an object with a state stored by the serialize process needs a cross check that the object is reseeded correctly when unserialized? That an inherent crash path needs to be plugged is a given, but all I am seeing in the proposed patches are more bandaids on the whole process? If the constructor can now be skipped in some instances then why have it at all? Go back to the situation where one manually called a 'constructor' when it was needed or fix the unserialize process on objects to include a suitable constructor? If an object has been 'created' but not 'initialized' then a process needs to be run to make the object actually usable? That some users of PHP have a problem with the recent changes is more because they are using undocumented processes so while allowing them to continue using the hacks short term may be correct - reverting the changes - the correct path going forward is perhaps to document a correct process and block any problem paths? -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk