diff --git a/NEWS b/NEWS index bacb68f387d0..66f782f16c66 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,9 @@ PHP NEWS . Fixed incorrect internal pointer and foreach iterator positions when compacting arrays with holes. (Weilin Du) +- Date: + . Fix unserialization of Time\Duration. (timwolla) + - DOM: . Fixed use-after-free when re-constructing a DOMXPath whose php:function registrations are freed while still reachable from the cycle collector. diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 31995eb80752..d13e40b12d11 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -182,6 +182,8 @@ PHP 8.6 INTERNALS UPGRADE NOTES instead of a zval*. Accordingly, zend_get_closure_this_ptr() now returns that zend_object*, or NULL when the closure is unbound, instead of a zval* that is IS_UNDEF when the closure is unbound. + . object_properties_load() now verifies that the given value is assignable + to typed properties. The check is performed in non-strict mode. - Added: . New zend_class_entry.ce_flags2 and zend_function.fn_flags2 fields were diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 43e21cafd564..3b4c49535b16 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1794,6 +1794,10 @@ ZEND_API void object_properties_load(zend_object *object, const HashTable *prope return; } } + if (ZEND_TYPE_IS_SET(property_info->type) && !zend_verify_property_type(property_info, prop, /* strict */ false)) { + return; + } + zval_ptr_dtor(slot); ZVAL_COPY_VALUE(slot, prop); zval_add_ref(slot); diff --git a/ext/date/tests/time/duration/serialize.phpt b/ext/date/tests/time/duration/serialize.phpt new file mode 100644 index 000000000000..f504a03e2396 --- /dev/null +++ b/ext/date/tests/time/duration/serialize.phpt @@ -0,0 +1,71 @@ +--TEST-- +Time\Duration: serialize() +--FILE-- +negate())); +echo f($unserialized = unserialize($serialized)), PHP_EOL; +var_dump(serialize($unserialized)); +echo f($unserialized->add($unserialized)), PHP_EOL; + +try { + // $negative is not bool, but coercible. + echo f(unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1;s:8:"negative";i:999;}')), PHP_EOL; +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +try { + // $negative is not bool and not coercible. + unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1;s:8:"negative";N;}'); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +try { + // $seconds is negative. + unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:-1;s:11:"nanoseconds";i:1;s:8:"negative";b:0;}'); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +try { + // Dynamic property. + unserialize('O:13:"Time\Duration":4:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1;s:8:"negative";b:0;s:3:"foo";N;}'); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + + +try { + // Out of range nanoseconds + unserialize('O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:1000000000;s:8:"negative";b:0;}'); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +try { + Time\Duration::fromSeconds(1, 1) + ->__unserialize([ + 'seconds' => 2, + 'nanoseconds' => 2, + 'negative' => true, + ]); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +string(85) "O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:2;s:8:"negative";b:1;}" + -1.000000002 +string(85) "O:13:"Time\Duration":3:{s:7:"seconds";i:1;s:11:"nanoseconds";i:2;s:8:"negative";b:1;}" + -2.000000004 + -1.000000001 +Exception: Invalid serialization data for Time\Duration object +Exception: Invalid serialization data for Time\Duration object +Exception: Invalid serialization data for Time\Duration object +Exception: Invalid serialization data for Time\Duration object +Exception: Invalid serialization data for Time\Duration object diff --git a/ext/date/time.stub.php b/ext/date/time.stub.php index b9d0a01b39d6..61ad5bc9e0cc 100644 --- a/ext/date/time.stub.php +++ b/ext/date/time.stub.php @@ -21,6 +21,10 @@ private function __construct() { } + public function __unserialize(array $data): void + { + } + public static function fromSeconds(int $seconds, int $nanoseconds = 0): Duration { } diff --git a/ext/date/time_arginfo.h b/ext/date/time_arginfo.h index b1dd72f4c24a..bdd4170450f8 100644 --- a/ext/date/time_arginfo.h +++ b/ext/date/time_arginfo.h @@ -1,9 +1,13 @@ /* This is a generated file, edit time.stub.php instead. - * Stub hash: b145db05ac54d90df4a1d5eb139c2d0c5e70401c */ + * Stub hash: 71349e499ff03529323bede7eb1e680637e81c85 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Time_Duration___construct, 0, 0, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Time_Duration___unserialize, 0, 1, IS_VOID, 0) + ZEND_ARG_TYPE_INFO(0, data, IS_ARRAY, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class_Time_Duration_fromSeconds, 0, 1, Time\\Duration, 0) ZEND_ARG_TYPE_INFO(0, seconds, IS_LONG, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, nanoseconds, IS_LONG, 0, "0") @@ -58,6 +62,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Time_Duration_compare, 0, ZEND_END_ARG_INFO() ZEND_METHOD(Time_Duration, __construct); +ZEND_METHOD(Time_Duration, __unserialize); ZEND_METHOD(Time_Duration, fromSeconds); ZEND_METHOD(Time_Duration, fromNanoseconds); ZEND_METHOD(Time_Duration, fromMicroseconds); @@ -75,6 +80,7 @@ ZEND_METHOD(Time_Duration, compare); static const zend_function_entry class_Time_Duration_methods[] = { ZEND_ME(Time_Duration, __construct, arginfo_class_Time_Duration___construct, ZEND_ACC_PRIVATE) + ZEND_ME(Time_Duration, __unserialize, arginfo_class_Time_Duration___unserialize, ZEND_ACC_PUBLIC) ZEND_ME(Time_Duration, fromSeconds, arginfo_class_Time_Duration_fromSeconds, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) ZEND_ME(Time_Duration, fromNanoseconds, arginfo_class_Time_Duration_fromNanoseconds, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) ZEND_ME(Time_Duration, fromMicroseconds, arginfo_class_Time_Duration_fromMicroseconds, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) diff --git a/ext/date/time_duration.c b/ext/date/time_duration.c index 0dab1ccb22f3..283dafe65dcd 100644 --- a/ext/date/time_duration.c +++ b/ext/date/time_duration.c @@ -77,11 +77,11 @@ static inline php_date_time_duration *create_duration_shell(zval *target) return Z_DATE_TIME_DURATION_P(target); } -ZEND_ATTRIBUTE_NODISCARD static inline zend_result sync_properties(php_date_time_duration *object) +static inline bool duration_representable(const timelib_duration *duration) { - if ( + return /* Check if the duration would overflow the $seconds property. */ - object->duration.seconds > ((uint64_t)ZEND_LONG_MAX) + duration->seconds <= ((uint64_t)ZEND_LONG_MAX) /* This constraint is an explicit part of PHP's API: It is the maximum $seconds * value that allows storing the entire duration as a single int64_t counting * nanoseconds, which might be desirable in the future when userland `int` is @@ -89,8 +89,12 @@ ZEND_ATTRIBUTE_NODISCARD static inline zend_result sync_properties(php_date_time * * While it is currently also enforced by timelib, this might change * in a future version of timelib, thus we also enforce it manually. */ - || object->duration.seconds > UINT64_C(9223372035) - ) { + && duration->seconds <= UINT64_C(9223372035); +} + +ZEND_ATTRIBUTE_NODISCARD static inline zend_result sync_properties(php_date_time_duration *object) +{ + if (!duration_representable(&object->duration)) { throw_out_of_range_exception(); return FAILURE; } @@ -149,6 +153,51 @@ PHP_METHOD(Time_Duration, __construct) zend_throw_error(NULL, "Cannot directly construct Time\\Duration, use Time\\Duration::from*() methods instead"); } +PHP_METHOD(Time_Duration, __unserialize) +{ + php_date_time_duration *duration = Z_DATE_TIME_DURATION_P(ZEND_THIS); + + HashTable *data; + + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_ARRAY_HT(data); + ZEND_PARSE_PARAMETERS_END(); + + object_properties_load(&duration->std, data); + if (EG(exception)) { + goto to_generic_error; + } + + zval *seconds = OBJ_PROP_NUM(&duration->std, 0); + zval *nanoseconds = OBJ_PROP_NUM(&duration->std, 1); + zval *negative = OBJ_PROP_NUM(&duration->std, 2); + + /* Verify that both properties are positive, since the timelib_duration_ctor_static() takes unsigned. */ + if (Z_LVAL_P(seconds) < 0 || Z_LVAL_P(nanoseconds) < 0) { + zend_throw_exception_ex(NULL, 0, "Invalid serialization data for %s object", ZSTR_VAL(duration->std.ce->name)); + RETURN_THROWS(); + } + + int error = timelib_duration_ctor_static(&duration->duration, Z_LVAL_P(seconds), Z_LVAL_P(nanoseconds), Z_TYPE_P(negative) == IS_TRUE); + if (error != TIMELIB_ERROR_NO_ERROR) { + throw_timelib_error(error); + goto to_generic_error; + } + + if (!duration_representable(&duration->duration)) { + throw_out_of_range_exception(); + goto to_generic_error; + } + + return; + + to_generic_error: + + /* Wrap any errors thrown by existing checks into a generic error. */ + zend_throw_exception_ex(NULL, 0, "Invalid serialization data for %s object", ZSTR_VAL(duration->std.ce->name)); + RETURN_THROWS(); +} + PHP_METHOD(Time_Duration, fromSeconds) { zend_ulong seconds;