php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Request #76774 Disable the ability to use concrete types in PHAR metadata
Submitted: 2018-08-21 16:21 UTC Modified: 2019-04-17 05:38 UTC
Votes:3
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: korvinszanto at gmail dot com Assigned: bishop (profile)
Status: Assigned Package: PHAR related
PHP Version: 7.2.9 OS: Ubuntu 18.04
Private report: No CVE-ID: None
View Developer Edit
 [2018-08-21 16:21 UTC] 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

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-04-17 05:38 UTC] bishop@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: bishop
 [2019-04-17 05:40 UTC] 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
 [2019-04-17 05:41 UTC] 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!
 [2020-08-05 00:36 UTC] tandre@php.net 
> 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)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 15:01:30 2024 UTC