# Unserializes, inheritance and allows_classes - Thread: https://externals.io/message/103756 - Original archive: https://news-web.php.net/php.internals/103756 - Started by Marcos Passos on 2019-01-17T23:48:51+00:00 - Messages: 9 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #103756 — Marcos Passos — 2019-01-17T23:48:51+00:00 URL: https://externals.io/message/103756#103756 Hi internals, Today I stumbled upon a limitation when implementing the unserialize method of a serializable class which depends on an abstraction also serializable. Currently, there is no way to unserialize an object specifying a parent class in the allowed_classes option: class SerializableBase implements \Serializable { > } > class SerializableChild extends SerializableBase { > } > class Foo implements \Serializable { > private $dependency; > public function __construct(SerializableBase $dependency) { > $this->dependency = $dependency; > } > public function `serialize()` : string { > return \serialize($this->dependency); > } > public function unserialize($data) : void { > $this->dependency = \unserialize($data, ['allowed_classes' => > SerializableBase::class]); > } > } Is this an intentional limitation? - Marcos --- ## #103757 — Marco Pivetta — 2019-01-18T00:24:06+00:00 URL: https://externals.io/message/103756#103757 · In reply to: #103756 On Fri, Jan 18, 2019 at 12:49 AM Marcos Passos wrote: > Hi internals, > > Today I stumbled upon a limitation when implementing the unserialize method > of a serializable class which depends on an abstraction also serializable. > Currently, there is no way to unserialize an object specifying a parent > class in the allowed_classes option: > > class SerializableBase implements \Serializable { > > […] > > > Is this an intentional limitation? > > Seems expected to me: `allowed_classes` is a whitelist, not a complex filter/ruleset. Also: nothing denies an attacker from defining a subtype to your class, then passing a malicious instance to your application. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/ --- ## #103758 — Marcos Passos — 2019-01-18T00:50:08+00:00 URL: https://externals.io/message/103756#103758 · In reply to: #103757 Hi Marco, Also: nothing denies an attacker from defining a subtype to your class, > then passing a malicious instance to your application. Fact, but it also reveals a fragility in the solution in the sense that one has to opt between flexible design or security. Em qui, 17 de jan de 2019 às 22:24, Marco Pivetta escreveu: > > On Fri, Jan 18, 2019 at 12:49 AM Marcos Passos > wrote: > > > […] > Seems expected to me: `allowed_classes` is a whitelist, not a complex > filter/ruleset. > > Also: nothing denies an attacker from defining a subtype to your class, > then passing a malicious instance to your application. > > Marco Pivetta > > http://twitter.com/Ocramius > > http://ocramius.github.com/ --- ## #103759 — Marco Pivetta — 2019-01-18T00:53:03+00:00 URL: https://externals.io/message/103756#103759 · In reply to: #103758 On Fri, Jan 18, 2019 at 1:50 AM Marcos Passos wrote: > Hi Marco, > > Also: nothing denies an attacker from defining a subtype to your class, > > […] > > > Fact, but it also reveals a fragility in the solution in the sense that > one has to opt between flexible design or security. > > Em qui, 17 de jan de 2019 às 22:24, Marco Pivetta > escreveu: > > > […] > Security is not a choice. The design is not fragile, it is strict and correct. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/ --- ## #103760 — Marcos Passos — 2019-01-18T01:00:48+00:00 URL: https://externals.io/message/103756#103760 · In reply to: #103759 How would you fix this example, then? Perhaps we should add more examples in the docs. Em qui, 17 de jan de 2019 às 22:53, Marco Pivetta escreveu: > On Fri, Jan 18, 2019 at 1:50 AM Marcos Passos > wrote: > > > […] > Security is not a choice. The design is not fragile, it is strict and > correct. > > Marco Pivetta > > http://twitter.com/Ocramius > > http://ocramius.github.com/ --- ## #103761 — Marco Pivetta — 2019-01-18T01:03:27+00:00 URL: https://externals.io/message/103756#103761 · In reply to: #103760 On Fri, Jan 18, 2019 at 2:00 AM Marcos Passos wrote: > How would you fix this example, then? Perhaps we should add more examples > in the docs. > You'd include all expected child classes in the list of `allowed_classes`. As for the docs, feel free to go ahead and edit them directly :+1: Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/ --- ## #103762 — Marcos Passos — 2019-01-18T01:13:36+00:00 URL: https://externals.io/message/103756#103762 · In reply to: #103761 > > How would you fix this example, then? Perhaps we should add more examples > in the docs. > But it closes for extension preventing the serialization of instances not whitelisted. It may work for @internal or package private classes, but not for public classes. Em qui, 17 de jan de 2019 às 23:03, Marco Pivetta escreveu: > > > On Fri, Jan 18, 2019 at 2:00 AM Marcos Passos > wrote: > > > […] > > You'd include all expected child classes in the list of `allowed_classes`. > As for the docs, feel free to go ahead and edit them directly :+1: > > Marco Pivetta > > http://twitter.com/Ocramius > > http://ocramius.github.com/ --- ## #103763 — Marco Pivetta — 2019-01-18T01:15:33+00:00 URL: https://externals.io/message/103756#103763 · In reply to: #103762 On Fri, Jan 18, 2019 at 2:13 AM Marcos Passos wrote: > How would you fix this example, then? Perhaps we should add more examples > > […] > > But it closes for extension preventing the serialization of instances not > whitelisted. It may work for @internal or package private classes, but not > for public classes. > That is precisely what this is designed for. A subclass is a different type, with a world of possible broken things in it: do not consider the subclass to be the same as a parent class: it's a logical mistake. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/ --- ## #103767 — Rowan Collins — 2019-01-18T14:30:53+00:00 URL: https://externals.io/message/103756#103767 · In reply to: #103763 > On Fri, Jan 18, 2019 at 2:13 AM Marcos Passos > wrote: > > […] > not > > […] > > That is precisely what this is designed for. > > A subclass is a different type, with a world of possible broken things in > it: do not consider the subclass to be the same as a parent class: it's a > logical mistake. > I think it depends on the scenario, and what risks you're trying to protect against. If you are using the whitelist in the core of a CMS, to stop plugins accidentally introducing problematic behaviour, sub-classes should absolutely be blocked; but if you're using it in a closed application to stop users manipulating the *serialized data*, there is no way for a sub-class to be created unless you create it yourself, so limiting to any class in a given hierarchy would be reasonable. However, you can always implement more complex logic by using custom serialization and deserializing with a factory which creates whichever sub-class is appropriate. Although fairly powerful, `serialize()` / deserialize() are always going to be a bit of a "lowest common denominator", rather than the right tool for every job. Regards, -- Rowan Collins [IMSoP]