On Tue, Jul 22, 2014 at 12:27 PM, Ferenc Kovacs <tyr...@gmail.com> wrote:
> sorry for the late reply. > you are right and after looking into the implementation I think classes > having custom object storage (eg. create_object) shouldn't assume that > their __construct will be called, but either do the initialization in the > create_object hook or validate in the other methods that the object was > properly initialized. > given that this lack of initialization problem is already present(you can > extend such a class and have a __construct() in the subclass which doesn't > call the parent constructor), and we want to keep the unserialize trick > fixed (as that exposes this problems to the remote attacker when some code > accepts arbitrary serialized data from the client) I see no reason to keep > the limitation in the ReflectionClass::newInstanceWithoutConstructor() and > allowing the instantiation of internal classes will provide a clean upgrade > path to doctrine and co. > ofc. we have to fix the internal classes misusing the constructor for > proper initialization one by one, but that can happen after the initial > 5.6.0 release (ofc it would be wonderful if people could lend me a hand for > fixing as much as we can before the release), but we have to fix those > anyways. > This sounds reasonable to me. newInstanceWithoutConstructor does not have the same remote exploitation concerns as serialize, so allowing crashes here that can also be caused by other means seems okay to me (especially if we're planning to fix them lateron). Only additional restriction I'd add is to disallow calling it on an internal + final class. For those the __construct argument does not apply. For them the entity-extending-internal-class usecase doesn't apply either, so that shouldn't be a problem. Nikita
