DatePeriod can crash if passed non initialized DateInterfaces

[arnaud-lb]

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 affected

Operating System

No response

[damianwadley]

==236214==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f7c20c68901 bp 0x7ffc67612760 sp 0x7ffc676125c8 T0)
==236214==The signal is caused by a READ memory access.
==236214==Hint: address points to the zero page.
    #0 0x7f7c20c68901 in memcpy (/lib/x86_64-linux-gnu/libc.so.6+0xc4901)
    #1 0x561795040fb4 in zim_DatePeriod___construct /home/ubuntu/php/php-8.1.20/src/ext/date/php_date.c:4347
    #2 0x561795e21dc7 in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /home/ubuntu/php/php-8.1.20/src/Zend/zend_vm_execute.h:1761
    #3 0x561795f68482 in execute_ex /home/ubuntu/php/php-8.1.20/src/Zend/zend_vm_execute.h:55807
    #4 0x561795f7b288 in zend_execute /home/ubuntu/php/php-8.1.20/src/Zend/zend_vm_execute.h:60151
    #5 0x561795d881d1 in zend_execute_scripts /home/ubuntu/php/php-8.1.20/src/Zend/zend.c:1846
    #6 0x561795c1aae3 in php_execute_script /home/ubuntu/php/php-8.1.20/src/main/main.c:2542
    #7 0x5617961569f4 in do_cli /home/ubuntu/php/php-8.1.20/src/sapi/cli/php_cli.c:965
    #8 0x561796158cae in main /home/ubuntu/php/php-8.1.20/src/sapi/cli/php_cli.c:1367
    #9 0x7f7c20bcdd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #10 0x7f7c20bcde3f in __libc_start_main_impl ../csu/libc-start.c:392
    #11 0x56179500e4c4 in _start (/home/ubuntu/php/php-8.1.20/bin/php+0x60e4c4)

[iluuu1994]

newInstanceWithoutConstructor is something... Maybe it should invoke some handler that allows internal objects to initialize internal data? We don't want to allocate a dummy time value in date_object_new_date, as it would be overwritten right away under normal conditions.

[arnaud-lb]

That's my reaction too. This also happens with subclasses that do not call the parent constructor.

The create_object handler 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]

This also happens with subclasses that do not call the parent constructor.

Right, that's even harder to catch... I wonder if it would be acceptable to disallow newInstanceWithoutConstructor for 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]

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]

For PHP 8.1 and up: #11919