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 functionserialize(): string {
return \serialize($this->dependency);
}
public function unserialize($data) : void {
$this->dependency = \unserialize($data, ['allowed_classes' =>
SerializableBase::class]);
}
}
Is this an intentional limitation?
- Marcos
On Fri, Jan 18, 2019 at 12:49 AM Marcos Passos marcospassos.com@gmail.com
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 {
}
class SerializableChild extends SerializableBase {
}
class Foo implements \Serializable {
private $dependency;
public function __construct(SerializableBase $dependency) {
$this->dependency = $dependency;
}
public functionserialize(): string {
return \serialize($this->dependency);
}
public function unserialize($data) : void {
$this->dependency = \unserialize($data, ['allowed_classes' =>
SerializableBase::class]);
}
}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
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 ocramius@gmail.com
escreveu:
On Fri, Jan 18, 2019 at 12:49 AM Marcos Passos marcospassos.com@gmail.com
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 {
}
class SerializableChild extends SerializableBase {
}
class Foo implements \Serializable {
private $dependency;
public function __construct(SerializableBase $dependency) {
$this->dependency = $dependency;
}
public functionserialize(): string {
return \serialize($this->dependency);
}
public function unserialize($data) : void {
$this->dependency = \unserialize($data, ['allowed_classes' =>
SerializableBase::class]);
}
}Is this an intentional limitation?
Seems expected to me:
allowed_classesis 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
On Fri, Jan 18, 2019 at 1:50 AM Marcos Passos marcospassos.com@gmail.com
wrote:
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 ocramius@gmail.com
escreveu:On Fri, Jan 18, 2019 at 12:49 AM Marcos Passos <
marcospassos.com@gmail.com> 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 {
}
class SerializableChild extends SerializableBase {
}
class Foo implements \Serializable {
private $dependency;
public function __construct(SerializableBase $dependency) {
$this->dependency = $dependency;
}
public functionserialize(): string {
return \serialize($this->dependency);
}
public function unserialize($data) : void {
$this->dependency = \unserialize($data, ['allowed_classes' =>
SerializableBase::class]);
}
}Is this an intentional limitation?
Seems expected to me:
allowed_classesis 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
Security is not a choice. The design is not fragile, it is strict and
correct.
Marco Pivetta
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 ocramius@gmail.com
escreveu:
On Fri, Jan 18, 2019 at 1:50 AM Marcos Passos marcospassos.com@gmail.com
wrote: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 ocramius@gmail.com
escreveu:On Fri, Jan 18, 2019 at 12:49 AM Marcos Passos <
marcospassos.com@gmail.com> 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 {
}
class SerializableChild extends SerializableBase {
}
class Foo implements \Serializable {
private $dependency;
public function __construct(SerializableBase $dependency) {
$this->dependency = $dependency;
}
public functionserialize(): string {
return \serialize($this->dependency);
}
public function unserialize($data) : void {
$this->dependency = \unserialize($data, ['allowed_classes' =>
SerializableBase::class]);
}
}Is this an intentional limitation?
Seems expected to me:
allowed_classesis 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
Security is not a choice. The design is not fragile, it is strict and
correct.Marco Pivetta
On Fri, Jan 18, 2019 at 2:00 AM Marcos Passos marcospassos.com@gmail.com
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
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 ocramius@gmail.com
escreveu:
On Fri, Jan 18, 2019 at 2:00 AM Marcos Passos marcospassos.com@gmail.com
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
On Fri, Jan 18, 2019 at 2:13 AM Marcos Passos marcospassos.com@gmail.com
wrote:
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.
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
On Fri, Jan 18, 2019 at 2:13 AM Marcos Passos marcospassos.com@gmail.com
wrote: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.
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]