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 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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
