Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110856
To: "internals@lists.php.net" <internals@lists.php.net>
CC: Bishop Bettini <bishop@php.net>, Stanislav Malyshev <smalyshev@gmail.com>
Thread-Topic: Including "Disable the ability to use concrete types in PHAR
 metadata" in PHP 8.0?
Thread-Index: AQHWU/GRw5eqUwR3jE+1bsRmt6W8gg==
Date: Tue, 7 Jul 2020 00:58:25 +0000
Message-ID:
 <DM6PR07MB6618AE5466C12EC544F527F7F9660@DM6PR07MB6618.namprd07.prod.outlook.com>
Accept-Language: en-CA, en-US
Content-Language: en-CA
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: d4a6a6f6-ad0d-470c-47d6-08d82210dae3
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jul 2020 00:58:25.9334
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Internet
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6NAM10HT123
Subject: Including "Disable the ability to use concrete types in PHAR
 metadata" in PHP 8.0?
From: tysonandre775@hotmail.com (tyson andre)

Hi internals,=0A=
=0A=
https://bugs.php.net/bug.php?id=3D76774 has been open since 2018-08-21.=0A=
=0A=
That ticket proposes the following:=0A=
=0A=
> I propose that we disable the ability to have concrete types included in =
the serialized metadata by=0A=
> providing an empty classlist to the unserialize call in the PHAR package.=
=0A=
> This will support the real cases we see in the wild of metadata usage whi=
ch is only array key values.=0A=
=0A=
A major change such as PHP 8.0 seems like a good time to disable this.=0A=
(but it seems safe enough for any minor version)=0A=
=0A=
Various blog posts have been written explaining the resulting vulnerabiliti=
es,=0A=
such as https://www.ixiacom.com/company/blog/exploiting-php-phar-deserializ=
ation-vulnerabilities-part-1 =0A=
=0A=
This change was previously proposed in https://externals.io/message/105271#=
105303=0A=
=0A=
> Bishop Bettini wrote,=0A=
>=0A=
> I agree that $allowed_classes is a partial fix.=0A=
> But is it not better to incrementally add defensive layers?=0A=
>=0A=
> I'll get to the immediate mitigation after I finish my phar fuzzing work,=
=0A=
> unless somebody beats me to it.=0A=
=0A=
I'm in favor of adding the defensive layer, and could probably implement th=
e immediate mitigation if needed.=0A=
=0A=
Thoughts on whether this needs an RFC? Has anything changed since that emai=
l thread? There seemed to be some debate over implementation details, but m=
ost responses considered the existing unserialization behavior problematic.=
=0A=
=0A=
- If it did, this may need to start less than two weeks after finishing an =
RFC, due to the feature freeze in august 8th.=0A=
=0A=
I assume the limitation of not allowing any objects (i.e. $allowed_classes=
=3D[]) for metadata would consistently affect getMetadata() and anything us=
ing the phar stream wrapper for a phar file.=0A=
Emitting an E_WARNING may be helpful but not absolutely necessary if an obj=
ect is seen anywhere in the data passed to Phar->setMetadata().=0A=
=0A=
Thanks,=0A=
- Tyson=