php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #75111 Memory disclosure or DoS via crafted .bmp image
Submitted: 2017-08-24 03:53 UTC Modified: 2017-08-24 11:49 UTC
From: glen dot carmichael at gmail dot com Assigned: cmb (profile)
Status: Closed Package: GD related
PHP Version: 7.2.0beta3 OS:
Private report: No CVE-ID: None
 [2017-08-24 03:53 UTC] glen dot carmichael at gmail dot com
Description:
------------
This is a security vulnerability in GD affecting imagecreatefromstring for PHP >= 7.2.

The BMP header format includes the offset in the file where image data begins. GD seeks to this offset when reading BMP images (in gd_bmp.c):

gdSeek(infile, header->off);

File I/O uses fseek, which is safe. Dynamic pointer I/O implements seek itself but fails to do proper bounds-checking. Specifically, dynamicSeek(ctx, pos) doesn't check if pos is negative. A negative pos corrupts the internal state, and the next read or write will be applied to an address before the buffer in memory.

By crafting a malicious BMP header, we can read (width * height * 3) bytes of memory at address (buffer + offset), where offset is any negative integer. Memory disclosure requires that the read doesn't segfault and the rendered image is displayed back to the user somehow (with memory encoded as RGB values).

Test script:
---------------
<?php

// craft BMP image
$str  = hex2bin("424D3603000000000000");
$str .= pack("V", -0x120000);   // offset of image data
$str .= pack("V", 40);          // length of header
$str .= pack("V", 256);         // width
$str .= pack("V", 256);         // height
$str .= hex2bin("01001800000000000000000000000000000000000000000000000000");

$im = imagecreatefromstring($str);
imagepng($im, "out.png");

?>

Expected result:
----------------
A negative offset should cause imagecreatefromstring to fail gracefully.

Actual result:
--------------
Depending on the memory layout, the script will either:

* cause a segfault
* cause a bus error
* succeed with 200kB of memory written to out.png as RGB values

Patches

fix-75111 (last revision 2017-08-24 10:53 UTC by cmb@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-08-24 09:31 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2017-08-24 10:52 UTC] cmb@php.net
-Status: Assigned +Status: Analyzed
 [2017-08-24 10:52 UTC] cmb@php.net
Thanks for the fine bug report! Indeed, I can confirm this issue,
and going to attach a patch which is supposed to fix the bug. Can
you please verify?

Since this affects only PHP 7.2[1] which is still in beta phase,
this doesn't appear to be a security issue with regard to PHP.
However, external libgd is affected as well, and since it supports
BMP for a long time, this is a serious issues requiring a CVE ID.
Therefore, I suggest to keep this ticket as security issue. I'm
going to forward this to <security@libgd.org>.

[1] BMP support has only been added in
    <http://git.php.net/?p=php-src.git;a=commit;h=500b496>
 [2017-08-24 10:53 UTC] cmb@php.net
The following patch has been added/updated:

Patch Name: fix-75111
Revision:   1503572028
URL:        https://bugs.php.net/patch-display.php?bug=75111&patch=fix-75111&revision=1503572028
 [2017-08-24 11:08 UTC] glen dot carmichael at gmail dot com
Thanks for the quick response. I don't seem to have access to view the patch, but failing when pos < 0 in dynamicSeek() should be enough.
 [2017-08-24 11:49 UTC] cmb@php.net
-Type: Security +Type: Bug
 [2017-08-24 11:49 UTC] cmb@php.net
> I don't seem to have access to view the patch, but failing when
> pos < 0 in dynamicSeek() should be enough.

The relevant part of the patch is:

    --- a/ext/gd/libgd/gd_io_dp.c
    +++ b/ext/gd/libgd/gd_io_dp.c
    @@ -152,6 +152,9 @@ static int dynamicSeek (struct gdIOCtx *ctx, const int pos)
        dynamicPtr *dp;
        dpIOCtx *dctx;
     
    +	if (pos < 0) {
    +		return FALSE;
    +	}
        dctx = (dpIOCtx *) ctx;
        dp = dctx->dp;

While investigating the issue for external libgd, I've noticed
that there is no security issue due to
<https://github.com/libgd/libgd/commit/4859d69> because *reading*
would fail if the position is negative. IMO it's still a bug to
allow to seek to negative positions, though.

Anyhow, since external libgd is not affected security-wise, and
since this is a low severity issue according to
<https://wiki.php.net/security>, I'm publicly disclosing this
report.

Thanks for having reported this issue before it could possibly
have affected production systems.
 [2017-08-24 12:08 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=5cd348c1d606b890abae076a38e47effcfda79be
Log: Fixed bug #75111 (Memory disclosure or DoS via crafted .bmp image)
 [2017-08-24 12:08 UTC] cmb@php.net
-Status: Analyzed +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 09:01:32 2024 UTC