zendframework / zend-serializer

Serializer component from Zend Framework
BSD 3-Clause "New" or "Revised" License
45 stars 19 forks source link

PhpSerialize: Add options for a unserialize() class-whitelist #36

Closed MatthiasKuehneEllerhold closed 6 years ago

MatthiasKuehneEllerhold commented 6 years ago

The PhpSerialize adapter uses unserialize() (see PhpSerialize.php#L88) to convert a string into a PHP Object (array, string, class-instance. ...).

PHP 7.0 added an option to only allow the deserialization of whitelisted classes via the "options['allowed_classes']" parameter to combat code-injection.

I'm proposing adding an options to PhpSerialize class to uses this whitelist.

Practical application: this could be used in "zend-i18n" (using "zend-cache" and "zend-serializer") for the translation-cache. It should only allow the deserialization of objects that are instance of Zend\I18n\Translator\TextDomain (or plain arrays).

Ocramius commented 6 years ago

to combat code-injection.

I fully agree with this, but changing the interface would make this serializer unusable with most applications, as subclassing and customising components is (sadly) very common.

MatthiasKuehneEllerhold commented 6 years ago

How about we add a setter (and a getter) and use the current (unsafe) whitelist as default? Example:

$serializer = new PhpSerialize();

// disallow all classes:
$serialize->setUnserializeWhitelist(null);

// OR: only allow the Zend-I18n-Textdomain:
$serialize->setUnserializeWhitelist([\Zend\I18n\Translator\TextDomain::class]);

$serialize->unserialize(...)
Ocramius commented 6 years ago

Instead of a setter, a ctor argument.

samsonasik commented 6 years ago

should be close-able in favor of merged #37

MatthiasKuehneEllerhold commented 6 years ago

Yes. PR #37 was merged! Thanks!