Everything

php

Built-in classes that cannot be serialized

php.internals

Sebastian Bergmann

7 years ago

The following classes register zend_class_serialize_deny: Closure, COM,
DOTNET, Generator, HashContext, Reflection, ReflectionClass,
ReflectionClassConstant, ReflectionExtension, ReflectionException,
ReflectionFunction, ReflectionFunctionAbstract, ReflectionGenerator,
ReflectionMethod, ReflectionNamedType, ReflectionObject,
ReflectionParameter, ReflectionProperty, ReflectionRype,
ReflectionZendExtension, SimpleXmlElement, SplFileInfo, Variant.

Are these all the built-in classes that cannot be serialized?

Would it be possible to implement ReflectionClass::isSerializable() that
returns false when the class is a) built-in and b) has
zend_class_serialize_deny registered?

Christoph Becker

7 years ago

On 26.11.2018 at 10:27, Sebastian Bergmann wrote:

> The following classes register zend_class_serialize_deny: Closure, COM,
> DOTNET, Generator, HashContext, Reflection, ReflectionClass,
> ReflectionClassConstant, ReflectionExtension, ReflectionException,
> ReflectionFunction, ReflectionFunctionAbstract, ReflectionGenerator,
> ReflectionMethod, ReflectionNamedType, ReflectionObject,
> ReflectionParameter, ReflectionProperty, ReflectionRype,
> ReflectionZendExtension, SimpleXmlElement, SplFileInfo, Variant.
>
> Are these all the built-in classes that cannot be serialized?

Well, the com_dotnet classes have only been made unserializable very
recently, due to bug #77177, so I can imagine that there are more
classes.  Particularly, the database related classes come to mind.  For
instance, it's currently possible to serialize SQLite3 instances, albeit
that makes no sense ("O:7:"SQLite3":0:{}").

> Would it be possible to implement ReflectionClass::isSerializable() that
> returns false when the class is a) built-in and b) has
> zend_class_serialize_deny registered?

I presume that this could easily be implemented, but if the method
returns TRUE for all other cases, it would be misleading since classes
may unconditionally throw in __sleep().

--
Christoph M. Becker

Nikita Popov

7 years ago

On Mon, Nov 26, 2018 at 10:28 AM Sebastian Bergmann <sebastian@php.net>
wrote:

> The following classes register zend_class_serialize_deny: Closure, COM,
> DOTNET, Generator, HashContext, Reflection, ReflectionClass,
> ReflectionClassConstant, ReflectionExtension, ReflectionException,
> ReflectionFunction, ReflectionFunctionAbstract, ReflectionGenerator,
> ReflectionMethod, ReflectionNamedType, ReflectionObject,
> ReflectionParameter, ReflectionProperty, ReflectionRype,
> ReflectionZendExtension, SimpleXmlElement, SplFileInfo, Variant.
>
> Are these all the built-in classes that cannot be serialized?
>
> Would it be possible to implement ReflectionClass::isSerializable() that
> returns false when the class is a) built-in and b) has
> zend_class_serialize_deny registered?
>

Apart from serialize_deny, a pretty common pattern is throwing __wakeup.
See for example CURLFile.

We should migrate such cases to serialize_deny though. I think it's pretty
weird to explicitly implement __wakeup (signalling that yes, you can be
unserialized), and then use it to throw (sorry, I lied).

In any case, what's your motivation here? As long as throwing
(un)serialize/__sleep/__wakeup exist, you will not be able to determine
whether a class can be (un)serialized a priori. It may even be that a class
can only sometimes be serialized. The only reliable way to find out is to
actually try it. What prevents you from attempting (un)serialization and
catching potentially thrown exceptions?

Nikita

Sebastian Bergmann

7 years ago

Am 26.11.2018 um 12:20 schrieb Nikita Popov:

> Apart from serialize_deny, a pretty common pattern is throwing __wakeup.
> See for example CURLFile.
>
> We should migrate such cases to serialize_deny though. I think it's pretty
> weird to explicitly implement __wakeup (signalling that yes, you can be
> unserialized), and then use it to throw (sorry, I lied).
>
> In any case, what's your motivation here? As long as throwing
> (un)serialize/__sleep/__wakeup exist, you will not be able to determine
> whether a class can be (un)serialized a priori. It may even be that a class
> can only sometimes be serialized. The only reliable way to find out is to
> actually try it. What prevents you from attempting (un)serialization and
> catching potentially thrown exceptions?

If I can rely on classes to throw an exception when serialize() is
performed (be it through zend_class_serialize_deny or in __sleep())
then, yes, I can just try it.

I, too, think that classes that currently throw in __sleep() should be
migrated to use zend_class_serialize_deny.

Only if all classes that cannot be serialized use
zend_class_serialize_deny then it would make sense to expose that
information through the Reflection API.

Nikita Popov

7 years ago

On Mon, Nov 26, 2018 at 12:27 PM Sebastian Bergmann <sebastian@php.net>
wrote:

> Am 26.11.2018 um 12:20 schrieb Nikita Popov:
> > Apart from serialize_deny, a pretty common pattern is throwing __wakeup.
> > See for example CURLFile.
> >
> > We should migrate such cases to serialize_deny though. I think it's
> pretty
> > weird to explicitly implement __wakeup (signalling that yes, you can be
> > unserialized), and then use it to throw (sorry, I lied).
> >
> > In any case, what's your motivation here? As long as throwing
> > (un)serialize/__sleep/__wakeup exist, you will not be able to determine
> > whether a class can be (un)serialized a priori. It may even be that a
> class
> > can only sometimes be serialized. The only reliable way to find out is to
> > actually try it. What prevents you from attempting (un)serialization and
> > catching potentially thrown exceptions?
>
> If I can rely on classes to throw an exception when serialize() is
> performed (be it through zend_class_serialize_deny or in __sleep())
> then, yes, I can just try it.
>

I believe you can rely on this. Not on any specific exception type, but the
fact that it will throw.

> I, too, think that classes that currently throw in __sleep() should be
> migrated to use zend_class_serialize_deny.
>

I've switched CURLFile, PDO and PDOStatement over to use serialize_deny. I
couldn't find other classes in bundled extensions that were manually
throwing on serialization.

> Only if all classes that cannot be serialized use
> zend_class_serialize_deny then it would make sense to expose that
> information through the Reflection API.
>

We could do that, but I'm not sure how useful it really is. After all, even
if a class can in principle be serialized, it might still have a property
that contains a class that cannot be serialized. Serializability is not a
property of a single class or object, it's a property of the whole object
graph that is being serialized.

Nikita

Sebastian Bergmann

7 years ago

Am 26.11.2018 um 13:35 schrieb Nikita Popov:

>> If I can rely on classes to throw an exception when serialize() is
>> performed (be it through zend_class_serialize_deny or in __sleep())
>> then, yes, I can just try it.
>
> I believe you can rely on this. Not on any specific exception type, but the
> fact that it will throw.

Good to know, thanks.

> I've switched CURLFile, PDO and PDOStatement over to use serialize_deny. I
> couldn't find other classes in bundled extensions that were manually
> throwing on serialization.

Thanks!

> that contains a class that cannot be serialized. Serializability is not a
> property of a single class or object, it's a property of the whole object
> graph that is being serialized.

I know:
https://github.com/sebastianbergmann/global-state/blob/master/src/Snapshot.php#L360
;-)

Best,
Sebastian

Stas Malyshev

7 years ago

Hi!

> We should migrate such cases to serialize_deny though. I think it's pretty
> weird to explicitly implement __wakeup (signalling that yes, you can be
> unserialized), and then use it to throw (sorry, I lied).

Throwing in __wakeup does not signal that it can be serialized. What it
says that if you try to unserialize it (note that the code doing
unserialize is not the same that does serialize and has no control over
what the argument string says - it may demand to unserialize anything)
it won't work. That _implies_ you shouldn't also serialize it (because
what's the point) but the important part is not to produce broken
objects from unserialization loop.

Also, for CURLFile for example there are additional things that happen
on __wakeup besides throwing, probably for security reasons. I am not
sure whether they are necessary anymore as we pretty much tell people
"don't unserialize external data" but they are there for now. Just
moving to _deny handler would probably not keep them.

--
Stas Malyshev
smalyshev@gmail.com

Nikita Popov

7 years ago

On Mon, Nov 26, 2018 at 11:47 PM Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> Hi!
>
> > We should migrate such cases to serialize_deny though. I think it's
> pretty
> > weird to explicitly implement __wakeup (signalling that yes, you can be
> > unserialized), and then use it to throw (sorry, I lied).
>
> Throwing in __wakeup does not signal that it can be serialized. What it
> says that if you try to unserialize it (note that the code doing
> unserialize is not the same that does serialize and has no control over
> what the argument string says - it may demand to unserialize anything)
> it won't work. That _implies_ you shouldn't also serialize it (because
> what's the point) but the important part is not to produce broken
> objects from unserialization loop.
>
> Also, for CURLFile for example there are additional things that happen
> on __wakeup besides throwing, probably for security reasons. I am not
> sure whether they are necessary anymore as we pretty much tell people
> "don't unserialize external data" but they are there for now. Just
> moving to _deny handler would probably not keep them.
>

Historically, __wakeup() has been the correct way to prevent
unserialization and/or mitigate issues relating to dangerous unserialized
state. The reason is that it was possible to bypass the unserialize_deny
handler by using the O-style, rather than the C-style serialization format.
At some point this whole was plugged and we don't allow O-unserializing
classes that have serialize/unserialize handlers.

Which is why nowadays, (un)serialize_deny is our strongest defense against
unserialization vulnerabilities, because it prevents unserialization
*before* the object is constructed. In the __wakeup case the object is
created first and __wakeup is only called much later at the end of
unserialization, which creates a lot more opportunities for the usual
shenanigans.

Basically, anything using a throwing __wakeup() nowadays is a leftover from
times where unserialize_deny was not properly enforced. To the best of my
knowledge, there is no good reason to use the throwing __wakeup pattern
nowadays anymore.

Regards,
Nikita