On Tue, Jul 21, 2020 at 3:37 PM tyson andre <tysonandre...@hotmail.com> wrote:
> Hi internals, > > I've started the vote on > https://wiki.php.net/rfc/phar_stop_autoloading_metadata > as announced earlier in https://externals.io/message/110871 > ([RFC] Don't automatically unserialize Phar metadata outside getMetadata()) > > This adds the mitigations described in > https://externals.io/message/105271#105291 , > which seemed to be the most straightforward approach to avoiding > unexpected side effects of unserialization. > > - For a trusted phar, I wouldn't expect to need to unserialize metadata to > check for the file not being corrupt > (e.g. there's a checksum, and people would have tested the phar > manually). > - For an untrusted phar, I'd want php to avoid calling unserialize() when > reading it through stream wrappers. > > https://bugs.php.net/bug.php?id=76774 goes into more detail about the > security issues this aims to fix. > > Thanks, > - Tyson > As a minor suggestion: > Additionally, add an $allowed_classes parameter to both getMetadata() implementations, defaulting to the current behavior of allowing any classes (true). This will be passed to the call to unserialize() performed internally. Rather than adding an $allowed_classes parameter, I'd add a general $unserialize_options parameter that just gets passed through to unserialize. E.g. we also have a "max_depth" option, which also seems potentially useful. This will ensure that any new limitations we implement for unserialize() will also be available in this context. Nikita
