Req #76774 [Com]: Disable the ability to use concrete types in PHAR metadata

Edit report at https://bugs.php.net/bug.php?id=76774&edit=1 ID: 76774 Comment by: tandre@php.net Reported by: korvinszanto at gmail dot com Summary: Disable the ability to use concrete types in PHAR metadata Status: Assigned Type: Feature/Change Request Package: PHAR related Operating System: Ubuntu 18.04 PHP Version: 7.2.9 Assigned To: bishop Block user comment: N Private report: N New Comment: > The immediate mitigation is to defer metadata deserialization until such time as it's > explicitly needed -- eg Phar::getMetadata(). Additionally, update the documentation to note the risk > of calling getMetadata on a phar from untrusted sources. Deferring metadata unserialization was implemented in https://wiki.php.net/rfc/phar_stop_autoloading_metadata - https://www.php.net/manual/en/phar.getmetadata.php still does not mention that calling getMetadata will be potentially risky in 8.0 (where previously just opening the phar or using phar streams caused the risk) Previous Comments: ------------------------------------------------------------------------ [2019-04-17 05:41:26] bishop@php.net <?php // Original code by Sam Thomas - @_s_n_t // @see https://cdn2.hubspot.net/hubfs/3853213/us-18-Thomas-It's-A-PHP-Unserialization-Vulnerability-Jim-But-Not-As-We-....pdf class TestObject { function __destruct() { echo "DESTRUCT!\n"; } } $phar = new Phar("phar.phar"); $phar->startBuffering(); $phar->addFromString("test.txt","test"); $phar->setStub("<?php __HALT_COMPILER(); ?>"); $o = new TestObject(); $phar->setMetadata($o); $phar->stopBuffering(); echo(file_get_contents("phar://phar.phar/test.txt")); // DESTRUCT! ------------------------------------------------------------------------ [2019-04-17 05:40:59] bishop@php.net As described on the PHP internals mailing list[1], phar deserializes header metadata upon opening the phar stream. If the metadata contains a serialized value that's meant to act maliciously[2], there's no way for the caller to prevent the malice[3]. The immediate mitigation is to defer metadata deserialization until such time as it's explicitly needed -- eg Phar::getMetadata(). Additionally, update the documentation to note the risk of calling getMetadata on a phar from untrusted sources. Future mitigations may make it possible to treat phar from untrusted sources with more care. Any such work, however, is out of scope for this ticket. [1]: https://marc.info/?l=php-internals&m=155525459217668&w=2 [2]: https://i.blackhat.com/us-18/Thu-August-9/us-18-Thomas-Its-A-PHP-Unserialization-Vulnerability-Jim-But-Not-As-We-Know-It.pdf [3]: https://cdn2.hubspot.net/hubfs/3853213/us-18-Thomas-It's-A-PHP-Unserialization-Vulnerability-Jim-But-Not-As-We-....pdf ------------------------------------------------------------------------ [2018-08-21 16:21:54] korvinszanto at gmail dot com Description: ------------ It's currently possible to serialize concrete types in a phar metadata, which opens us up to the potential for deserialization attacks through gadget chains. Since PHAR files can serialize concrete types, they can serialize chains of objects that, when deserialized, fire __construct __destruct and __wakeup. This is an issue largely because PHP will automatically deseralize this payload in the following case: file_exists('phar://path/to/phar') which has cauused RCE in a couple projects in the wild including Typo3 https://typo3.org/security/advisory/typo3-core-sa-2018-002/ 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. Test script: --------------- <?php class Foo { public function __wakeup() { die('FIRED WAKEUP UNEXPECTEDLY'); } } $file = './test.phar'; if (!file_exists($file)) { $p = new Phar($file); $p->setStub(Phar::createDefaultStub(__FILE__)); $p->addFile(__FILE__); // pointing main file which requires all classes $p->setMetadata(new Foo()); $p->compress(Phar::GZ); } // Trigger unsafe deserialization file_exists($file); Expected result: ---------------- File_exists currently loads the entire metadata for the phar. If this is intended, I'd expect the metadata to safely unserialize without fire any magic methods. Actual result: -------------- Magic methods get fired ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=76774&edit=1