Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100213
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.51 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c-PumhFT0spXrLfEDZ-R3pY+HRAnuX+2m_aOu=RJ6qOJQ@mail.gmail.com>
References: <CAF+90c9-irX2gCu_858nQwQ-ARshwbV_8mtNAN-UMBgSvXVp0w@mail.gmail.com>
 <f59a0ba5-228b-88ed-bb99-d296684726c1@gmail.com> <CAF+90c_RUYwFgXK6rYvspvm1w+MUaeimRVzMJzHyWVj_t3oYXA@mail.gmail.com>
 <CAF+90c-PumhFT0spXrLfEDZ-R3pY+HRAnuX+2m_aOu=RJ6qOJQ@mail.gmail.com>
Date: Tue, 15 Aug 2017 11:29:50 +0200
Message-ID: <CAF+90c_wia6tzOTY3RV+X=BWOfmyBjO-JpTN_omX++8r9ikLOg@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="94eb2c111d12d2bbe80556c76b7c"
Subject: Re: [PHP-DEV] Unserialize security policy
From: nikita.ppv@gmail.com (Nikita Popov)

--94eb2c111d12d2bbe80556c76b7c
Content-Type: text/plain; charset="UTF-8"

On Fri, Aug 11, 2017 at 12:55 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> On Thu, Aug 10, 2017 at 10:49 AM, Nikita Popov <nikita.ppv@gmail.com>
> wrote:
>
>> On Sun, Aug 6, 2017 at 12:49 AM, Stanislav Malyshev <smalyshev@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
>

Here's some external perspective on unserialize() security by Sean Heelan:
https://sean.heelan.io/2017/08/12/fuzzing-phps-unserialize-function/

The two main points are:
1. While it's true that if you're using unserialize() on untrusted input
you are most likely going to be vulnerable due to object injection, it may
be quite hard for an attacker to exploit this for closed source
applications, because it requires knowledge about specific classes defined
by the application. On the other hand, bugs in unserialize() itself can be
exploited much more reliably, without knowing any specific details of the
application.
2. We should be able to precipitate most unserialize() bugs by regularly
fuzzing it ourselves. The setup for doing so is also provided.

Nikita

--94eb2c111d12d2bbe80556c76b7c--