Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110872
MIME-Version: 1.0
References: <DM6PR07MB6618AE5466C12EC544F527F7F9660@DM6PR07MB6618.namprd07.prod.outlook.com>
In-Reply-To: <DM6PR07MB6618AE5466C12EC544F527F7F9660@DM6PR07MB6618.namprd07.prod.outlook.com>
Reply-To: bishop@php.net
Date: Tue, 7 Jul 2020 21:55:33 -0400
Message-ID: <CAEYWF=4hStv1erow6ZLAXvRHCrxiVrQMR8ZqgTznciePjBKcug@mail.gmail.com>
To: tyson andre <tysonandre775@hotmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000065feee05a9e4695a"
Subject: Re: Including "Disable the ability to use concrete types in PHAR
 metadata" in PHP 8.0?
From: bishop@php.net (Bishop Bettini)

--00000000000065feee05a9e4695a
Content-Type: text/plain; charset="UTF-8"

On Mon, Jul 6, 2020 at 8:58 PM tyson andre <tysonandre775@hotmail.com>
wrote:

> Hi internals,
>
> https://bugs.php.net/bug.php?id=76774 has been open since 2018-08-21.
>
> That ticket proposes the following:
>
> > I propose that we disable the ability to have concrete types included in
> the serialized metadata by
> > providing an empty classlist to the unserialize call in the PHAR package.
> > This will support the real cases we see in the wild of metadata usage
> which is only array key values.
>
> A major change such as PHP 8.0 seems like a good time to disable this.
> (but it seems safe enough for any minor version)
>
> Various blog posts have been written explaining the resulting
> vulnerabilities,
> such as
> https://www.ixiacom.com/company/blog/exploiting-php-phar-deserialization-vulnerabilities-part-1
>
> This change was previously proposed in
> https://externals.io/message/105271#105303
>
> > Bishop Bettini wrote,
> >
> > I agree that $allowed_classes is a partial fix.
> > But is it not better to incrementally add defensive layers?
> >
> > I'll get to the immediate mitigation after I finish my phar fuzzing work,
> > unless somebody beats me to it.
>
> I'm in favor of adding the defensive layer, and could probably implement
> the immediate mitigation if needed.
>
> Thoughts on whether this needs an RFC? Has anything changed since that
> email thread? There seemed to be some debate over implementation details,
> but most responses considered the existing unserialization behavior
> problematic.
>
> - If it did, this may need to start less than two weeks after finishing an
> RFC, due to the feature freeze in august 8th.
>

Thanks for raising this.

I agree that 8.0 is a great time to fix it. While I see this as a design
oversight rather than a new feature - and don't think it requires an RFC -
the RFC provides welcome visibility into the pros and cons. So, +1.

I've been working on an IMAP issue for some time now[1], and Phar work has
been sidelined as a result. Fortunately, the fix is straightforward and I'm
happy to work together to land this before feature freeze.

[1]:https://wiki.php.net/todo/ext/imap/xoauth2

--00000000000065feee05a9e4695a--