date: Fix unserialization of Time\Duration - #23629
Conversation
| zval *seconds = OBJ_PROP_NUM(&duration->std, 0); | ||
| zval *nanoseconds = OBJ_PROP_NUM(&duration->std, 1); | ||
| zval *negative = OBJ_PROP_NUM(&duration->std, 2); | ||
| /* object_properties_load() does not type check. We need to do this ourselves. */ |
f3c7903 to
170fb17
Compare
| zval *negative = OBJ_PROP_NUM(&duration->std, 2); | ||
| /* object_properties_load() does not type check. We need to do this ourselves. */ | ||
| if (Z_TYPE_P(seconds) != IS_LONG || Z_TYPE_P(nanoseconds) != IS_LONG || (Z_TYPE_P(negative) != IS_FALSE && Z_TYPE_P(negative) != IS_TRUE)) { | ||
| zend_throw_exception_ex(NULL, 0, "Invalid serialization data for %s object", ZSTR_VAL(duration->std.ce->name)); |
There was a problem hiding this comment.
So you typed check, after the properties have been overwritten, but that leaves the object in an unexpected type-violating state AFAICT?
There was a problem hiding this comment.
Keep in mind people can call ->__unserialize manually BTW.
There was a problem hiding this comment.
but that leaves the object in an unexpected type-violating state AFAICT?
That is correct, but I don't think this situation is observable in any way: Duration is a final readonly internal class, thus the engine prevents constructing objects without going through the constructor (or unserialization).
There are thus two cases to reach __unserialize():
- Manually calling
->__unserialize()on an existing object: This is guaranteed to fail due toobject_properties_load()rejecting the reassignment of thereadonlyproperties. - Unserializing a payload.
In the second case, the types could mismatch after object_properties_load(), but I don't think there is a way this broken object can be observed. It will cleanly be destructed at the end of the failed unserialize() call (which is save since Duration doesn't have a custom destructor and particularly no destructor that accesses the properties).
3 participants
No description provided.