Repository navigation
DatePeriod can crash if passed non initialized DateInterfaces #11416
Description
Description
Found these while working on lazy objects.
The following code snippets result in crashes:
PHP Version
PHP 8.1
<?php
$date = (new ReflectionClass(DateTime::class))->newInstanceWithoutConstructor();
new DatePeriod($date, new DateInterval('P1D'), 2);PHP Version
PHP 8.2
<?php
$date = (new ReflectionClass(DateTime::class))->newInstanceWithoutConstructor();
$dateperiod = (new ReflectionClass(DatePeriod::class))->newInstanceWithoutConstructor();
$dateperiod->__unserialize(['start' => $date]); // __wakeup is also affectedOperating System
No response
Activity
damianwadley commentedon Jun 9, 2023
iluuu1994 commentedon Jun 22, 2023
newInstanceWithoutConstructoris something... Maybe it should invoke some handler that allows internal objects to initialize internal data? We don't want to allocate a dummytimevalue indate_object_new_date, as it would be overwritten right away under normal conditions.arnaud-lb commentedon Jun 24, 2023
That's my reaction too. This also happens with subclasses that do not call the parent constructor.
The
create_objecthandler is always called, but the best it can do without access to constructor arguments is to initialize the object internal state some defined values.All objects check if their constructor has been called before accessing their internal state (like this in ext/date, but that's not specific to ext/date) and throw an exception otherwise, but it's easy to forget about adding this check, especially when the object is not $this.
iluuu1994 commentedon Jun 24, 2023
Right, that's even harder to catch... I wonder if it would be acceptable to disallow
newInstanceWithoutConstructorfor internal classes that have state other than properties, as there would be no way of initializing this from userland without calling the constructor, so might as well initialize the object like normal.arnaud-lb commentedon Jun 24, 2023
One way to prevent this currently is to declare a class as final and not-serializable. This disallows
newInstanceWithoutConstructor, sub-classing, and unserialize() (which is an other way to instantiate without calling constructor).However, this may break some use-cases. E.g. test mocks or lazy objects rely on being able to sub-class and to not call the constructor. Mocking date objects is probably not a good practice, but there may be good reasons to mock some classes like PDO in tests.
derickr commentedon Aug 9, 2023
For PHP 8.1 and up: #11919
Fix phpGH-11416: Crash with DatePeriod when uninitialised objects are…
Fix phpGH-11416: Crash with DatePeriod when uninitialised objects are…