On Thu, Aug 10, 2017 at 10:49 AM, Nikita Popov <nikita....@gmail.com> wrote:
> On Sun, Aug 6, 2017 at 12:49 AM, Stanislav Malyshev <smalys...@gmail.com> > wrote: > >> Hi! >> >> > https://bugs.php.net/bug.php?id=75006 has been marked as a non-security >> > bug, with the justification that unserialize() should not be fed >> untrusted >> > input. While we do document that unserialize() shouldn't be used on >> > untrusted input, we have always treated these as security bugs in the >> past. >> >> Not always, but sometimes we did. I think we should stop doing it, as to >> not validate the idea that unserialize can safely be used with untrusted >> data (it can't, and it doesn't look likely that it ever will be, at >> least not without comprehensive rewrite and possibly removing references >> support, which is not likely to happen). >> >> If anybody strongly feels that this is wrong, we can make an RFC about >> it, but given the current state of unserialize(), I can not say we can >> support such usage scenario in the current state of unserialize() code, >> and would like to hear arguments to the contrary. >> >> If somebody wants to do something about it, please feel welcome, we have >> a number of open unserialize bugs right now (if you want to work on them >> and don't have access to private bugs and you believe you should - >> please ask on security@ list). >> > > Thanks everyone for the clarification. I agree that this is the right > decision. I think it would be good to update the security policy to > explicitly mention unserialize(), as this is probably our largest source of > security bug reports right now, so there's bound to be questions from > security researchers regarding this. > > Nikita > 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. Nikita
