Re: Unserialize security policy

On 11.08.2017 at 12:55, Nikita Popov wrote: > I think it might also be useful to make a distinction based on > allowed_classes here. I think there is a reasonable expectation that if > allowed_classes is empty (and as such any object injection vectors are > excluded), unserialize() should be safe. The vast majority of unserialize() > bugs are variants on the theme of __wakeup() and > Serializable::unserialize(). But there are also bugs that apply with > allowed_classes=>[], for example ùßâÆjä6J/™P–?Ó¸ > https://bugs.php.net/bug.php?id=75054. What about references? Consider, for instance, the following code: <?php $_POST['untrusted_input'] = 'a:1:{i:0;a:1:{i:0;R:2;}}'; function flatten($array) { if (is_array($array)) { $result = []; foreach ($array as $element) { $result = array_merge($result, flatten($element)); } return $result; } return [$array]; } $unserializedInput = unserialize($_POST['untrusted_input'], []); flatten($unserializedInput); Of course, the flatten() function is naive, but it is fine for any "normal" input. However, this very code has a DOS issue. Do we really want to say that it is the developers responsibility to check for infinite recursion for code that uses the result of unserialize(…, []) in this way? It appears to me that unserialize() cannot ever be safe, unless class instantiation *and* references can be excluded. (Neither of these "features" are available in JSON or (supposed to be) in WDDX, by the way.) While the former is possible, the latter is not (yet), so in my humble opinion we should not try to claim that unserialize(…, []) is safe, at least unless there is a mechanism to disallow unserializing of references, too. -- Christoph M. Becker