php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #77269 efree() on uninitialized Heap data in imagescale leads to use-after-free
Submitted: 2018-12-09 08:50 UTC Modified: 2019-01-07 08:09 UTC
From: sscannell at ripstech dot com Assigned: stas (profile)
Status: Closed Package: GD related
PHP Version: 5.6.39 OS: Linux
Private report: No CVE-ID: 2016-10166
 [2018-12-09 08:50 UTC] sscannell at ripstech dot com
Description:
------------
When malicious user input is passed to the imagescale function of the gd extension, efree() will be called on uninitialized heap data. This can lead to
use after free vulnerabilities if the heap is sprayed with the address of a known structure.

The bug occurs in ext/gd/libgd/gd_interpolation.c in the function _gdContributionsAlloc(int line_size, int windows_size). The function will attempt to allocate helper structs and receives two parameters: the line size and the windows size. To prevent integer overflows, each parameter is passed to gd's overflow2() function before being used in the gdMalloc function.
(gdMalloc is just #define gdMalloc emalloc).

However, if the overflow2 check for windows size is positive, overflow_error is set to true, which leads to gd attempting to free all the lines allocated so far. The issue is that gd does not check if any lines have been allocated so far at all. By supplying input that leads to overflow2 being true, .Weights is freed, which is an unintialized pointer. 

if (overflow2(line_length, sizeof(ContributionType))) {
		gdFree(res);
		return NULL;
	}
	res->ContribRow = (ContributionType *) gdMalloc(line_length * sizeof(ContributionType));
	if (res->ContribRow == NULL) {
		gdFree(res);
		return NULL;
	}
	for (u = 0 ; u < line_length ; u++) {
		if (overflow2(windows_size, sizeof(double))) {
			overflow_error = 1;
		} else {
			res->ContribRow[u].Weights = (double *) gdMalloc(windows_size * sizeof(double));
		}
		if (overflow_error == 1 || res->ContribRow[u].Weights == NULL) {
			unsigned int i;
			u--;
			for (i=0;i<=u;i++) {
                gdFree(res->ContribRow[i].Weights);
			}


When the for loop is reached that frees the uninitialized pointers, i will be 0 and u too. However, before the for loop is entered u is decremented by one so it will turn into -1 , which leads to the condition i <=0 never being met. 

I have run the test script provided on my Debian production server with the latest version of php installed (7.0 line). The free() error occurs but the execution continues. I am not sure if that is because of the distribution or PHP version. The same code persists in php 7.2.13 and 5.6.39 although I have not managed to crash the 5.6.39 line so far






Test script:
---------------
<?php
$img = imagecreate(pow(2, 27), 0x01);
var_dump(imagescale($img, 0x01, 0x01, 20));
echo "Execution continues!\n";


Expected result:
----------------
No Crash

Actual result:
--------------
php fault.php 
bool(false)
Execution continues!
*** Error in `php': free(): invalid pointer: 0x00005597c21f2d80 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x70bfb)[0x7fd6fa8f2bfb]
/lib/x86_64-linux-gnu/libc.so.6(+0x76fc6)[0x7fd6fa8f8fc6]
/lib/x86_64-linux-gnu/libc.so.6(+0x7780e)[0x7fd6fa8f980e]
/usr/lib/x86_64-linux-gnu/libgd.so.3(gdImageDestroy+0x71)[0x7fd6f61bf921]
php(+0x271141)[0x5597c1282141]
php(list_entry_destructor+0x1a)[0x5597c12821ca]
php(zend_hash_index_del+0xc6)[0x5597c127e076]
php(zend_hash_graceful_reverse_destroy+0x111)[0x5597c127eea1]
php(shutdown_executor+0x42b)[0x5597c125cbbb]
php(zend_deactivate+0x6b)[0x5597c126cebb]
php(php_request_shutdown+0x296)[0x5597c120a9f6]
php(+0x2f28b0)[0x5597c13038b0]
php(main+0x46e)[0x5597c10edebe]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7fd6fa8a22e1]
php(_start+0x2a)[0x5597c10ee00a]
======= Memory map: ========
5597c1011000-5597c13bb000 r-xp 00000000 fe:01 5781                       /usr/bin/php7.0
5597c15ba000-5597c1630000 r--p 003a9000 fe:01 5781                       /usr/bin/php7.0
5597c1630000-5597c1640000 rw-p 0041f000 fe:01 5781                       /usr/bin/php7.0
5597c1640000-5597c165c000 rw-p 00000000 00:00 0 
5597c20b5000-5597c2227000 rw-p 00000000 00:00 0                          [heap]
7fd6e4000000-7fd6e4021000 rw-p 00000000 00:00 0 
7fd6e4021000-7fd6e8000000 ---p 00000000 00:00 0 
7fd6f1c28000-7fd6f1c2c000 r-xp 00000000 fe:01 131865                     /usr/lib/php/20151012/tokenizer.so
7fd6f1c2c000-7fd6f1e2b000 ---p 00004000 fe:01 131865                     /usr/lib/php/20151012/tokenizer.so
7fd6f1e2b000-7fd6f1e2c000 r--p 00003000 fe:01 131865                     /usr/lib/php/20151012/tokenizer.so
7fd6f1e2c000-7fd6f1e2d000 rw-p 00004000 fe:01 131865                     /usr/lib/php/20151012/tokenizer.so
7fd6f1e2d000-7fd6f1e30000 r-xp 00000000 fe:01 131863                     /usr/lib/php/20151012/sysvshm.so
...

Patches

fix-77269 (last revision 2018-12-12 15:56 UTC by cmb@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-12-09 08:56 UTC] stas@php.net
I imagine this should be reported to libgd maintainers?
 [2018-12-09 12:07 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2018-12-09 12:07 UTC] cmb@php.net
I'll investigate whether upstream is affected as well, and will
forward in this case.
 [2018-12-09 13:21 UTC] cmb@php.net
This issue has been fixed upstream long ago[1], but
obviously I forgot to port the fix downstream.  Sorry!

@sscannell Can you please verify that the issue is resolved with
this patch[2]?

BTW: it seems to me that the width of the original image has to be
at least 2**28; 2**27 won't trigger an overflow.

Anyhow, there still appears to be a bug, since with the fix
applied, imagescale() returns an image resource, but it should
actually fail.  I'll have a look at this, and follow up with
another ticket, if appropriate.

[1] <https://github.com/libgd/libgd/commit/60bfb401ad5a4a8ae995dcd36372fe15c71e1a35>
[2] <https://gist.github.com/cmb69/1ce784d6631cd36c58ef4de8919e1198>
 [2018-12-09 21:31 UTC] sscannell at ripstech dot com
Hi,

Ahh, I see. The patch does seem to do the trick! 
Yes, the scale should have been pow(2, 28). That was on me because I got confused: 
I made this seg fault with pow(2, 28) on my testing environment (compiled 7.2.13) and was curious what would happen on a real server. It seems that this server already had the downstream patch applied but the free bug still occured, but only if it was pow(2, 27). I thought the reason for that was some difference in the overflow calculation since the odds that I accidentally found another free() related bug was pretty small.

I can't read the related bug report, if you give me access I can help figure this out as I am sure that this is still security related.
 [2018-12-09 22:19 UTC] cmb@php.net
-Status: Assigned +Status: Analyzed
 [2018-12-09 22:19 UTC] cmb@php.net
> The patch does seem to do the trick! 

Great.  Thanks for the quick reply, and for reporting the issue in
the first place!  I'll provide a full-fledged formatted patch as
soon as possible.

> I thought the reason for that was some difference in the
> overflow calculation […]

Indeed, upstream libgd 2.2.5 still has an issue regarding the
overflow check; it is too restrictive, which has been fixed in
upstream's master[1].

> I can't read the related bug report, if you give me access I can
> help figure this out as I am sure that this is still security
> related.

If you are referring to bug 77272: this is not security related,
but rather the follow-up bug (after my patch has been applied)
which returns a (blank) image resource instead of false (since
_gdScaleHoriz() and _gdScaleVert() are void functions, and as such
don't indicate failure).  I'm keeping the ticket private since the
test script is the same as in this report.

[1] <https://github.com/libgd/libgd/commit/c3cf674cb444696a36f720f785878b41225af063>
 [2018-12-12 15:56 UTC] cmb@php.net
The following patch has been added/updated:

Patch Name: fix-77269
Revision:   1544630174
URL:        https://bugs.php.net/patch-display.php?bug=77269&patch=fix-77269&revision=1544630174
 [2018-12-12 15:57 UTC] cmb@php.net
-Assigned To: cmb +Assigned To: stas -CVE-ID: +CVE-ID: 2016-10166
 [2018-12-12 15:57 UTC] cmb@php.net
Stas, can you please take over from here?
 [2018-12-16 00:45 UTC] stas@php.net
Sure, I can merge the patch.
 [2018-12-30 02:41 UTC] stas@php.net
@cmb: I am getting the failure in imagecreate() on the test, not imagescale. Is that intended or something is wrong?
 [2018-12-30 02:43 UTC] stas@php.net
-PHP Version: 7.0.33 +PHP Version: 5.6.39
 [2018-12-30 02:43 UTC] stas@php.net
In security repo as 097091cfb3e8bb6e41d546b97ee562607c260f69.
 [2018-12-30 14:06 UTC] cmb@php.net
> I am getting the failure in imagecreate() on the test, not
> imagescale. Is that intended or something is wrong?

Ah, you're likely hitting the overly restricted overflow check in
gdImageCreate which has been fixed for PHP 7.2.0 only[1], and is
not yet fixed in any upstream libgd release (but only in master).
It may not be possible to trigger the imagescale() related bug with
this overly restricted check.

I suggest to commit the *test* *file* only to PHP-7.2 and up, but
the fix to PHP-5.6 and up (better safe than sorry).  Also the test
needs a skip condition for upstream libgd builds[2]; I can apply
this later, since there are already several tests which lack such
checks[3].

[1] <http://git.php.net/?p=php-src.git;a=commit;h=c60cdac636f3a25f0300538aa3dfb60803e888a1>
[2] <https://gist.github.com/cmb69/1f36d285eb297ed326f5c821d7aafced#file-patch-L42>
[3] <https://bugs.php.net/72557>
 [2018-12-30 19:38 UTC] stas@php.net
I've added modified test to 5.6 for now (just in case), but will use the original one when upmerging to 7.2. Thanks for explaining it!
 [2019-01-07 08:10 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a918020c03880e12ac9f38e11a4a3789491a5f85
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:10 UTC] stas@php.net
-Status: Analyzed +Status: Closed
 [2019-01-07 08:19 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dfd8237aec01116b32447881aa6008a90d45cb4c
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:19 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a918020c03880e12ac9f38e11a4a3789491a5f85
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:20 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dfd8237aec01116b32447881aa6008a90d45cb4c
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:20 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a918020c03880e12ac9f38e11a4a3789491a5f85
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:20 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dfd8237aec01116b32447881aa6008a90d45cb4c
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:20 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a918020c03880e12ac9f38e11a4a3789491a5f85
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:21 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=dfd8237aec01116b32447881aa6008a90d45cb4c
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 08:21 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a918020c03880e12ac9f38e11a4a3789491a5f85
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 [2019-01-07 13:17 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c1edfc748b88ef025edd23553888536ed62dc38e
Log: Fix #77269: Potential unsigned underflow in gdImageScale
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Tue Dec 10 02:01:24 2019 UTC