Not sure how to upload attachment, but you can find test.gif from http://zyang.co.uk/test.gif

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.

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.

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?

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.

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.

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().

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.

Great! Thanks for your time!
If there will be a note in the document, please let me know.

The documentation regarding this issue has been improved:
<http://svn.php.net/viewvc?view=revision&revision=343841>.

@zyyang Is that sufficient?

Hi,

In change log, can you add a short note like thanks for Zhouyuan Yang of Fortinet's FortiGuard Labs?

> 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.

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

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']);

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.

> 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).

Another Ref:
https://bugs.php.net/bug.php?id=75571