Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100179
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.41 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c_RUYwFgXK6rYvspvm1w+MUaeimRVzMJzHyWVj_t3oYXA@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>
Date: Fri, 11 Aug 2017 12:55:01 +0200
Message-ID: <CAF+90c-PumhFT0spXrLfEDZ-R3pY+HRAnuX+2m_aOu=RJ6qOJQ@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a11449eb41ec70d0556782503"
Subject: Re: [PHP-DEV] Unserialize security policy
From: nikita.ppv@gmail.com (Nikita Popov)

--001a11449eb41ec70d0556782503
Content-Type: text/plain; charset="UTF-8"

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

--001a11449eb41ec70d0556782503--