New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insecure PHP deserialization through phar:// wrapper. #949

Closed
ByQwert opened this Issue Jan 8, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@ByQwert
Copy link

ByQwert commented Jan 8, 2019

Hello,

During bug bounty hunting I met one of the old version of yours library and decided to test it for known vulnerabilities, namely PHP deserialization through phar:// wrapper that was discovered independently by @s-n-t and @orangetw.

Presentation Slides by Sam Thomas
White Paper by Sam Thomas
CTF challenge by Orange Tsai

So, after some tests, looks like yours library has similar issue as TCPDF library.

Method getImage() of Image/ImageProcessor class pass value of src attribute of img tag to fopen() function, what can lead to PHP deserialization if value contains phar:// wrapper.
https://github.com/mpdf/mpdf/blob/development/src/Image/ImageProcessor.php#L215

For proof of concept I create class with __destruct() method (based on GuzzleHttp deserialization chain) and use phpggc library to create image, which contains phar metadata. Phpggc repo also provide dozens of popular deserialization chains for popular frameworks and libraries.

Source code of test class:

<?php

namespace TestLib;

class VulnerableClass {

    private $file;
    private $content;

    public function __construct($file, $content) {
        $this->file = $file;
        $this->content = $content;
    }

    /* ... */

    public function __destruct() {
        $this->save($this->file, $this->content);
    }

    public function save($file, $content) {
        file_put_contents($file, $content);
    }
}

?>

Source code of trigger script:

<?php

require('lib/class.php');

require_once __DIR__ . '/../vendor/autoload.php';

$input = $_GET['input'];

if (!$input) {
    $input = "";
}

$mpdf = new \Mpdf\Mpdf();
$mpdf->WriteHTML($input);
$mpdf->Output();

?>

Video Proof Of Concept

Tested on version 5.4.0 and 7.1.7 (latest).
PHP 7.1.25

TCPDF CVE:
https://nvd.nist.gov/vuln/detail/CVE-2018-17057
TCPDF fix commit that disallow pass pathes with phar:// wrapper:
tecnickcom/TCPDF@1861e33

jakejackson1 added a commit to GravityPDF/mpdf that referenced this issue Jan 9, 2019

@finwe finwe closed this in #950 Jan 9, 2019

finwe added a commit that referenced this issue Jan 9, 2019

@finwe

This comment has been minimized.

Copy link
Member

finwe commented Jan 9, 2019

Thanks gentlement, for both the broad issue report and for the quick and elegant fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment