php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #75788 [FG-VD-18-005] PHP GD Denial of Service Vulnerability Notification
Submitted: 2018-01-10 05:22 UTC Modified: 2018-01-25 21:59 UTC
From: zyyang at fortinet dot com Assigned: cmb (profile)
Status: Closed Package: GD related
PHP Version: 5.6.33 OS: CentOS 6.8 x64
Private report: No CVE-ID: None
 [2018-01-10 05:22 UTC] zyyang at fortinet dot com
Description:
------------
Subject: [FG-VD-18-005] PHP GD Denial of Service Vulnerability Notification

-------

Vulnerability Notification
Jan 09, 2018
Tracking Case #: FG-VD-18-005

Dear PHP,

The following information pertains to information discovered by Fortinet's FortiGuard Labs. It has been determined that a vulnerability exists in PHP GD. To streamline the disclosure process, we have created a preliminary advisory which you can find below. This upcoming advisory is purely intended as a reference, and does not contain sensitive information such as proof of concept code. 

As a mature corporation involved in security research, we strive to responsibly disclose vulnerability information. We will not post an advisory until we determine it is appropriate to do so in co-ordination with the vendor unless a resolution cannot be reached. We will not disclose full proof of concept, only details relevant to the advisory.

We look forward to working closely with you to resolve this issue, and kindly ask for your co-operation during this time. Please let us know if you have any further questions, and we will promptly respond to address any issues. 

If this message is not encrypted, it is because we could not find your key to do so. If you have one available for use, please notify us and we will ensure that this is used in future correspondence. We ask you use our public PGP key to encrypt and communicate any sensitive information with us. You may find the key on our FortiGuard center at: http://www.fortiguard.com/pgpkey.

Type of Vulnerability & Repercussions:
	Denial of Service

Affected Product:
	PHP 5.6.32 (cli) (built: Oct 29 2017 19:00:01)
	GD Version bundled (2.1.0 compatible)
	FreeType Version 2.3.11
	libPNG Version 1.2.49
	libXpm Version 30411

Upcoming Advisory Reference:
	http://www.fortiguard.com/advisory/UpcomingAdvisories.html

Credits:
	This vulnerability was discovered by Zhouyuan Yang of Fortinet's FortiGuard Labs.

Proof of Concept & Additional Information:
	The issue exists in the "imagecopyresampled" function.

	To reproduce,
	1. Attacker modify an image file, for example, a gif file, set the Logical Screen Descriptor value to FF FF FF FF. Shown in figure 1.
	2. Execute this image file (test.gif) with the PoC test.php. Then the server CPU goes to 100% (for a better result, we can set the test server to single core processor or do it multiple times in a short time, like upload it 100 times in 5 seconds.) Shown in figure 2.
	
	For example, when an upload program using this function, with multi-thread uploading a tiny file (like 100 threads at the same time with the same payload, see figure 3, tested in the latest WordPress).
	Would cause a long time 100% usage with a quad-core processor. Even after the client side stops the attack.
	
	The error log shows:
	[Tue Jan 09 21:17:20 2018] [error] [client 192.168.56.1] PHP Fatal error:  Maximum execution time of 30 seconds exceeded in /var/www/html/uploads/test.php on line 18
	
	Not sure how to upload attachments.

	


Test script:
---------------
<?php
// The file
$filename = 'test.gif';


// Content type
header('Content-Type: image/gif');

// Get width height
list($width, $height) = getimagesize($filename);

// Resample

$image_p = imagecreatetruecolor(180, 180);
$image = imagecreatefromgif($filename);

// The issue
imagecopyresampled( $image_p, $image, 0, 0, 0, 0, 180, 180, $width, $height );
?>


Expected result:
----------------
Server CPU goes to 100%

Error Log:
[Tue Jan 09 21:17:20 2018] [error] [client 192.168.56.1] PHP Fatal error:  Maximum execution time of 30 seconds exceeded in /var/www/html/uploads/test.php on line 18

Actual result:
--------------
For example, when an upload program using this function, with multi-thread uploading a tiny file (like 100 threads at the same time with the same payload, see figure 3, tested in the latest WordPress).

Would cause a long time 100% usage with a quad-core processor. Even after the client side stops the attack.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-01-10 05:28 UTC] zyyang at fortinet dot com
Not sure how to upload attachment, but you can find test.gif from http://zyang.co.uk/test.gif
 [2018-01-10 14:22 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2018-01-10 16:49 UTC] cmb@php.net
-Status: Assigned +Status: Analyzed
 [2018-01-10 16:49 UTC] cmb@php.net
Thanks for reporting this issue.

The issue is not really related to GD, but rather to
getimagesize() which always returns the size given in the logical
screen descriptor of GIFs.  imagecreatefromgif(), however, creates
an image with the size given in the first image descriptor.  So in
this case $width and $height equal 65535, while the image has only
a size of 200✕200.  The following imagecopyresampled()
consequently takes a lot of time to downsize the image.

Actually, one might argue that this is not even a security issue,
since the proper script would be:

    <?php
    // The file
    $filename = 'test.gif';


    // Content type
    header('Content-Type: image/gif');

    // Resample

    $image_p = imagecreatetruecolor(180, 180);
    $image = imagecreatefromgif($filename);
 
    // Get width height
    $width = imagesx($image);
    $height = imagesy($image);

    // The issue
    imagecopyresampled( $image_p, $image, 0, 0, 0, 0, 180, 180, $width, $height );

I'm not sure whether getimagesize() should be "fixed" to return
the size given in the first image descriptor of GIF files, or
whether this issue should merely be documented.
 [2018-01-10 18:56 UTC] zyyang at fortinet dot com
Hi,

Thanks for your correction.
As I mentioned, in the real world, like WordPress or some image processing software which calls this function. I can always cause the CPU goes 100% for a long time, for example, uploading 100 images (just 2kb size) in a short time. Their engineer says they are using PHP internal functions, they have limited the file size, but they can't fix it on the code side (https://hackerone.com/reports/303626).

I am not familiar with the reason, but the result is causing a DoS attack. I think it's better to have some check function.
Some vulnerabilities like CVE-2017-0780, CVE-2017-2416 and CVE-2017-0478 may be helpful.
 [2018-01-11 20:49 UTC] stas@php.net
Not sure how this is security issue. If you accept arbitrary images from the internet and resize them, it can happen that somebody sends you a huge image that it takes a long time to resize it. Moreover, if your script accepts the resize parameters from, essentially, the user data, I assume the question how long the resize would take is controlled by the user too. To me it looks like if you accepted string and integer, then did str_repeat($string, $integer) and then complained that for some values of string and integer it could take a lot of time and a lot of memory. Am I missing something here?
 [2018-01-11 23:04 UTC] zyyang at fortinet dot com
Hi,

Yes, for a huge image, that it takes a long time to resize it.
But the issue here is a tiny file, shows in Windows 10 just 150*150 size, 3kb, would cause the processor goes 100% for a log of time. I think most of the site admin won't notice it before processing the file.
And if goes for multithreading, the result is worse (takes me 30 mins for 20 uploads), the server most likely goes offline.
 [2018-01-11 23:19 UTC] stas@php.net
The fact that file size on disk is tiny means nothing. Image formats use compression, tiny file can contain huge images, like other compressed formats, see https://en.wikipedia.org/wiki/Zip_bomb. You should be checking the actual variables you passing to processing function, not compressed file size on disk.
 [2018-01-11 23:36 UTC] zyyang at fortinet dot com
Good point.
But the issue is existing in a number of working PHP software, they all limited the file size and file width & height to try to avoid this kind of attack. But for the file width & height, some of them is getting from getimagesize(), some not.

I think we may consider adding a comment to the document, that the file width & height from the function getimagesize() is not same as the file width & height shows in some other ways. And when using the function imagecopyresampled(), it is preferred to get the image width and height using the function getimagesize().
 [2018-01-11 23:40 UTC] stas@php.net
The software that uses only file size and the software that checks different values than it sends to imagecopyresampled() should of course be fixed. But I am afraid we can't do much here to fix it, that should be done by the developers of that software. 

Since GIF can be not a single image but collection of images (e.g. animated GIFs), it is indeed a good idea to document which of the sizes each function returns.
 [2018-01-11 23:44 UTC] zyyang at fortinet dot com
Great! Thanks for your time!
If there will be a note in the document, please let me know.
 [2018-01-15 14:02 UTC] cmb@php.net
-Status: Analyzed +Status: Feedback
 [2018-01-15 14:02 UTC] cmb@php.net
The documentation regarding this issue has been improved:
<http://svn.php.net/viewvc?view=revision&revision=343841>.

@zyyang Is that sufficient?
 [2018-01-16 17:44 UTC] zyyang at fortinet dot com
-Status: Feedback +Status: Assigned
 [2018-01-16 17:44 UTC] zyyang at fortinet dot com
Hi,

In change log, can you add a short note like thanks for Zhouyuan Yang of Fortinet's FortiGuard Labs?
 [2018-01-16 20:06 UTC] stas@php.net
-Type: Security +Type: Documentation Problem
 [2018-01-17 12:24 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 [2018-01-17 12:24 UTC] cmb@php.net
> In change log, can you add a short note like thanks for Zhouyuan
> Yang of Fortinet's FortiGuard Labs?

Unfortunately, the SVN server is not configured to allow for prop
changes.
 [2018-01-25 01:46 UTC] zyyang at fortinet dot com
Hi,

I found a similar issue with this one. May help you reconsider it as a security issue, it happens on web servers and may let servers went down.

https://bugs.php.net/bug.php?id=66387
https://github.com/libgd/libgd/issues/213
 [2018-01-25 12:07 UTC] cmb@php.net
On hindsight, I fail to see why bug #66387 has been treated as
security issue for PHP/GD – actually, it would be only a security
issue for userland code which allows to pass nonsensical values to
a function.  That's comparable to something like

  while ($_GET['foo']);

or

  str_repeat('foo', $_GET['bar']);

or

  function fac($n) {
      return $n > 1 ? $n * fac($n - 1) : 1;
  }
  echo fac($_GET['num']);
 [2018-01-25 18:21 UTC] zyyang at fortinet dot com
Hi,

For a program running on a local host, it is not a security issue. Just some bad codes as you said.
For a program running on a web server, again, it could cause a DoS attack.
 [2018-01-25 18:39 UTC] cmb@php.net
> For a program running on a web server, again, it could cause a
> DoS attack.

Indeed.  The question is, however, whether this is something that
has to be fixed in PHP or GD, or something that has to be fixed in
respective userland code.  I think it is the latter; otherwise PHP
had to forbid recursive function calls or user input at all (see
the third example of my former post).
 [2018-01-25 21:11 UTC] zyyang at fortinet dot com
Another Ref:
https://bugs.php.net/bug.php?id=75571
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Fri Apr 19 12:01:25 2019 UTC