php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #72494 imagecropauto out-of-bounds access
Submitted: 2016-06-25 22:19 UTC Modified: 2016-10-25 20:41 UTC
From: fernando at null-life dot com Assigned: cmb (profile)
Status: Closed Package: GD related
PHP Version: 5.5.37 OS: *
Private report: No CVE-ID: None
 [2016-06-25 22:19 UTC] fernando at null-life dot com
Description:
------------
imagecropauto on IMG_CROP_THRESHOLD mode causes arbitrary read access and possible leak of information.

The function imagecropauto doesn't check valid colors for non-truecolor images. This causes that gdImageRed/Green/Blue/Alpha macros read beyond that 255 allowed values, when it troes match colors inside the gdColorMatch function.

I'm attaching a possible patch based on the verification that was present in another PHP/gd function but this verification could also be done inside gdImageCropThreshold. 


Source code:

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

color is only checked to be positive:

PHP_FUNCTION(imagecropauto)
{
...
		case GD_CROP_THRESHOLD:
			if (color < 0) {
				php_error_docref(NULL, E_WARNING, "Color argument missing with threshold mode");
				RETURN_FALSE;
			}
			im_crop = gdImageCropThreshold(im, color, (float) threshold);
			break;
...

color is passed to gdColorMatch:

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

gdImagePtr gdImageCropThreshold(gdImagePtr im, const unsigned int color, const float threshold)
{
...
	match = 1;
	for (y = 0; match && y < height; y++) {
		for (x = 0; match && x < width; x++) {
			match = (gdColorMatch(im, color, gdImageGetPixel(im, x,y), threshold)) > 0;
		}
	}
...


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

col1 (color) is now used with the gdImageRed/gdImageGreen/gdImageBlue/gdImageAlpha macros.

static int gdColorMatch(gdImagePtr im, int col1, int col2, float threshold)
{
	const int dr = gdImageRed(im, col1) - gdImageRed(im, col2);
	const int dg = gdImageGreen(im, col1) - gdImageGreen(im, col2);
	const int db = gdImageBlue(im, col1) - gdImageBlue(im, col2);
	const int da = gdImageAlpha(im, col1) - gdImageAlpha(im, col2);
	const double dist = sqrt(dr * dr + dg * dg + db * db + da * da);
	const double dist_perc = sqrt(dist / (255^2 + 255^2 + 255^2));
	return (dist_perc <= threshold);
	//return (100.0 * dist / 195075) < threshold;
}

In this macros it is used as an index (c) for the red/green/blue/alpha arrays, we are able to read beyond the 255 items on these arrays.

https://github.com/php/php-src/blob/master/ext/gd/libgd/gd.h#L730

#define gdImageColorsTotal(im) ((im)->colorsTotal)
#define gdImageRed(im, c) ((im)->trueColor ? gdTrueColorGetRed(c) : \
	(im)->red[(c)])
#define gdImageGreen(im, c) ((im)->trueColor ? gdTrueColorGetGreen(c) : \
	(im)->green[(c)])
#define gdImageBlue(im, c) ((im)->trueColor ? gdTrueColorGetBlue(c) : \
	(im)->blue[(c)])
#define gdImageAlpha(im, c) ((im)->trueColor ? gdTrueColorGetAlpha(c) : \
	(im)->alpha[(c)])



------
-----------------------------------------------------
GDB output:
 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_crop.c:350
Breakpoint 2 at 0x8190002: file /home/user/php/php-70/ext/gd/libgd/gd_crop.c, line 350.
gdb-peda$ r
Starting program: /home/user/php/php-70/sapi/cli/php -n poc.php
[----------------------------------registers-----------------------------------]
EAX: 0x0
EBX: 0x8b47000 --> 0x8b46db8 --> 0x1
ECX: 0x60c
EDX: 0x0
ESI: 0xf5a14020 --> 0xf5a7a118 --> 0x84b92de (<ZEND_DO_ICALL_SPEC_HANDLER>:     push   ebp)
EDI: 0xf5a7a118 --> 0x84b92de (<ZEND_DO_ICALL_SPEC_HANDLER>:    push   ebp)
EBP: 0xffff9f18 --> 0xffff9f78 --> 0xffff9fd8 --> 0xffffa008 --> 0xffffa028 --> 0xffffa068 (--> ...)
ESP: 0xffff9ee0 --> 0x4
EIP: 0x8190002 (<gdColorMatch+366>:     mov    eax,DWORD PTR [ebp-0x28])
EFLAGS: 0x246 (carry PARITY adjust ZERO sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x818fffb <gdColorMatch+359>:        sub    edx,eax
   0x818fffd <gdColorMatch+361>:        mov    eax,edx
   0x818ffff <gdColorMatch+363>:        mov    DWORD PTR [ebp-0x1c],eax
=> 0x8190002 <gdColorMatch+366>:        mov    eax,DWORD PTR [ebp-0x28]
   0x8190005 <gdColorMatch+369>:        imul   eax,DWORD PTR [ebp-0x28]
   0x8190009 <gdColorMatch+373>:        mov    edx,eax
   0x819000b <gdColorMatch+375>:        mov    eax,DWORD PTR [ebp-0x24]
   0x819000e <gdColorMatch+378>:        imul   eax,DWORD PTR [ebp-0x24]
[------------------------------------stack-------------------------------------]
0000| 0xffff9ee0 --> 0x4
0004| 0xffff9ee4 --> 0x1
0008| 0xffff9ee8 --> 0x4
0012| 0xffff9eec --> 0x0
0016| 0xffff9ef0 --> 0x0
0020| 0xffff9ef4 --> 0x0
0024| 0xffff9ef8 --> 0x0
0028| 0xffff9efc --> 0x0
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value

Breakpoint 2, gdColorMatch (im=0xf5a6c000, col1=0x539, col2=0x0, threshold=0) at /home/user/php/php-70/ext/gd/libgd/gd_crop.c:350
350             const double dist = sqrt(dr * dr + dg * dg + db * db + da * da);
gdb-peda$ p/d col1
$1 = 1337
gdb-peda$ p &im->red
$4 = (int (*)[256]) 0xf5a6c010  ## im->red bottom limit

gdb-peda$ p &im->red[255]
$6 = (int *) 0xf5a6c40c         ## im->red top limit
       
gdb-peda$ p &im->red[col1]
$3 = (int *) 0xf5a6d4f4         ## Out of bounds



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

$img = imagecreate(10,10);
imagecropauto($img, IMG_CROP_THRESHOLD, 0, 1337);

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

Actual result:
--------------
ASan ouput:
=================================================================
==23837==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf0d05df4 at pc 0x08681a2e bp 0xff8da798 sp 0xff8da788
READ of size 4 at 0xf0d05df4 thread T0
    #0 0x8681a2d in gdColorMatch /home/user/php/php-70-asan/ext/gd/libgd/gd_crop.c:348
    #1 0x8681a2d in gdImageCropThreshold /home/user/php/php-70-asan/ext/gd/libgd/gd_crop.c:253
    #2 0x85b9587 in zif_imagecropauto /home/user/php/php-70-asan/ext/gd/gd.c:4624
    #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 0xf6c7b636 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)

0xf0d05df4 is located 92 bytes to the right of 7320-byte region [0xf0d04100,0xf0d05d98)
allocated by thread T0 here:
    #0 0xf727ad06 in malloc (/usr/lib/i386-linux-gnu/libasan.so.2+0x96d06)
    #1 0x94b68b9 in _emalloc /home/user/php/php-70-asan/Zend/zend_alloc.c:2446

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/user/php/php-70-asan/ext/gd/libgd/gd_crop.c:348 gdColorMatch
Shadow bytes around the buggy address:
  0x3e1a0b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e1a0b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e1a0b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e1a0b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x3e1a0ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x3e1a0bb0: 00 00 00 fa fa fa fa fa fa fa fa fa fa fa[fa]fa
  0x3e1a0bc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e1a0bd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e1a0be0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e1a0bf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x3e1a0c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==23837==ABORTING

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-06-25 22:29 UTC] fernando at null-life dot com
I can't see if the patch was attached, I'm typing it as a comment here just in case:

diff --git a/ext/gd/gd.c b/ext/gd/gd.c
index d863523..8c88474 100644
--- a/ext/gd/gd.c
+++ b/ext/gd/gd.c
@@ -4617,7 +4617,7 @@ PHP_FUNCTION(imagecropauto)
                        break;

                case GD_CROP_THRESHOLD:
-                       if (color < 0) {
+                       if (color < 0 || (!gdImageTrueColor(im) && color >= gdImageColorsTotal(im))) {
                                php_error_docref(NULL, E_WARNING, "Color argument missing with threshold mode");
                                RETURN_FALSE;
                        }
 [2016-06-26 23:50 UTC] stas@php.net
-Assigned To: +Assigned To: pajoye
 [2016-06-26 23:50 UTC] stas@php.net
Looks like gdImageCropThreshold should validate the colors? Pierre, could you take a look?

Also doesn't look like security issue, but delaying that waiting Pierre's answer.
 [2016-06-27 01:40 UTC] pajoye@php.net
Hi,

Thanks for the detailed report :)

The fix looks correct however it should be done in gd_crop.c instead.

It should be applied to 5.5+

Can we/I commit directly or do we see it as critical? It does look not like critical to me and will allow to get it in the libgd next release.
 [2016-06-27 01:50 UTC] stas@php.net
-PHP Version: 7.0.8 +PHP Version: 5.5.37
 [2016-06-27 01:50 UTC] stas@php.net
Does not look critical to me. If you commit it to libgd, please feel free to merge directly for PHP, and close the bug once it's done.
 [2016-10-25 01:41 UTC] stas@php.net
-Assigned To: pajoye +Assigned To: cmb
 [2016-10-25 12:11 UTC] cmb@php.net
The patch suggested by Fernando has already been applied[1] to PHP
5.6.25 and PHP 7.0.10 and higher. gdImageCropThreshold() has
already been fixed as of libgd 2.2.3[2]. I'm going to apply this
fix against the bundled libgd and add a respective regression test
and NEWS entry.

[1] <http://git.php.net/?p=php-src.git;a=commit;h=1d69028d2f15216d128b5a6e606f763ef09d4991>
[2] <https://github.com/libgd/libgd/commit/b347e0342a138f01088f86983d31c6b22e19246f>
[3] <https://github.com/libgd/libgd/commit/46f2c690c682d5da33539034480c58c1a6d5cfac>
 [2016-10-25 12:53 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 08:01:29 2024 UTC