php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72709 imagesetstyle() causes OOB read for empty $styles
Submitted: 2016-07-29 22:13 UTC Modified: 2016-08-02 17:08 UTC
From: marceloje at gmail dot com Assigned: cmb
Status: Closed Package: GD related
PHP Version: 7.0.9 OS: Linux x86_64
Private report: No CVE-ID:
 [2016-07-29 22:13 UTC] marceloje at gmail dot com
Description:
------------
"imagesetstyle" does not check the number of elements of "style" array parameter. It can accept empty array, then "gdMalloc" allocates less bytes than necessary. Finally, "imagesetpixel" reads out of bounds for styled colors and it could have data leak through colors of image.

Source code:

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

PHP_FUNCTION(imagesetstyle)
{
        zval *IM, *styles, *item;
        gdImagePtr im;
        int *stylearr;
        int index = 0;

        if (zend_parse_parameters(ZEND_NUM_ARGS(), "ra", &IM, &styles) == FAILURE)  {
                return;
        }

        if ((im = (gdImagePtr)zend_fetch_resource(Z_RES_P(IM), "Image", le_gd)) == NULL) {
                RETURN_FALSE;
        }

        /* copy the style values in the stylearr */
        stylearr = safe_emalloc(sizeof(int), zend_hash_num_elements(Z_ARRVAL_P(styles)), 0);     // zend_hash_num_elements(Z_ARRVAL_P(styles)) = 0

        ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(styles), item) {
                stylearr[index++] = zval_get_long(item);
        } ZEND_HASH_FOREACH_END();

        gdImageSetStyle(im, stylearr, index);

        efree(stylearr);

        RETURN_TRUE;
}

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

void gdImageSetStyle (gdImagePtr im, int *style, int noOfPixels)
{
        if (im->style) {
                gdFree(im->style);
        }
        im->style = (int *) gdMalloc(sizeof(int) * noOfPixels);              // noOfPixels = 0, then allocated memory is less then necessary for any minimum style
        memcpy(im->style, style, sizeof(int) * noOfPixels);
        im->styleLength = noOfPixels;
        im->stylePos = 0;
}

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

void gdImageSetPixel (gdImagePtr im, int x, int y, int color)
{
        int p;
        switch (color) {
                case gdStyled:
                        if (!im->style) {
                                /* Refuse to draw if no style is set. */
                                return;
                        } else {
                                p = im->style[im->stylePos++];             // p read a integer out of bounds
                        }
                        if (p != gdTransparent) {
                                gdImageSetPixel(im, x, y, p);             // p is allocated as color from image
                        }
                        im->stylePos = im->stylePos % im->styleLength;
                        break;
...



GDB output:

USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0 gdb -q --args /home/operac/php-70-sinasan/sapi/cli/php -n poc.php
Reading symbols from /home/operac/php-70-sinasan/sapi/cli/php...done.
(gdb)  b php_gd_gdImageSetStyle
Breakpoint 1 at 0x55a406: file /home/operac/php-70-sinasan/ext/gd/libgd/gd.c, line 2811.
(gdb) r
Starting program: /home/operac/php-70-sinasan/sapi/cli/php -n poc.php

Breakpoint 1, php_gd_gdImageSetStyle (im=0x139df70, style=0x139ddb0, noOfPixels=0) at /home/operac/php-70-sinasan/ext/gd/libgd/gd.c:2811       // noOfPixels = 0
2811            if (im->style) {
(gdb) b _emalloc
Breakpoint 2 at 0x86d354: file /home/operac/php-70-sinasan/Zend/zend_alloc.c, line 2442.
(gdb) c
Continuing.

Breakpoint 2, _emalloc (size=0, __zend_filename=0xd3d688 "/home/operac/php-70-sinasan/ext/gd/libgd/gd.c", __zend_lineno=2814, __zend_orig_filename=0x0, __zend_orig_lineno=0) at /home/operac/php-70-sinasan/Zend/zend_alloc.c:2442 // reserved memory = 0
2442            if (UNEXPECTED(AG(mm_heap)->use_custom_heap)) {
...

Suggested patch:

diff --git a/ext/gd/gd.c b/ext/gd/gd.c
index 9375aee..90f4979 100644
--- a/ext/gd/gd.c
+++ b/ext/gd/gd.c
@@ -1444,6 +1444,7 @@ PHP_FUNCTION(imagesetstyle)
        gdImagePtr im;
        int *stylearr;
        int index = 0;
+       int nelem;

        if (zend_parse_parameters(ZEND_NUM_ARGS(), "ra", &IM, &styles) == FAILURE)  {
                return;
@@ -1453,6 +1454,12 @@ PHP_FUNCTION(imagesetstyle)
                RETURN_FALSE;
        }

+        nelem = zend_hash_num_elements(Z_ARRVAL_P(styles));
+        if (nelem <= 0) {
+                php_error_docref(NULL, E_WARNING, "You must have at least 1 color in your array style");
+                RETURN_FALSE;
+        }
+
        /* copy the style values in the stylearr */
        stylearr = safe_emalloc(sizeof(int), zend_hash_num_elements(Z_ARRVAL_P(styles)), 0);




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

$img = imagecreatetruecolor(1, 1);

imagesetstyle($img, array());
imagesetpixel($img, 0, 0, -2);

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

Actual result:
--------------
ASan output:

USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0 /ramdisk/php-fuzz/phuzzer/php-70/sapi/cli/php -n  poc.php
=================================================================
==27580==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000ab30 at pc 0x00000099703a bp 0x7ffd3b76e180 sp 0x7ffd3b76e170
READ of size 4 at 0x60200000ab30 thread T0
    #0 0x997039 in php_gd_gdImageSetPixel /home/operac/php-70/ext/gd/libgd/gd.c:734
    #1 0x957f1a in zif_imagesetpixel /home/operac/php-70/ext/gd/gd.c:3084
    #2 0x1da38da in ZEND_DO_ICALL_SPEC_HANDLER /home/operac/php-70/Zend/zend_vm_execute.h:586
    #3 0x1b4c335 in execute_ex /home/operac/php-70/Zend/zend_vm_execute.h:414
    #4 0x1df9dc8 in zend_execute /home/operac/php-70/Zend/zend_vm_execute.h:458
    #5 0x194764a in zend_execute_scripts /home/operac/php-70/Zend/zend.c:1427
    #6 0x16b8347 in php_execute_script /home/operac/php-70/main/main.c:2494
    #7 0x1e02126 in do_cli /home/operac/php-70/sapi/cli/php_cli.c:974
    #8 0x467378 in main /home/operac/php-70/sapi/cli/php_cli.c:1344
    #9 0x7fa9cc55782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #10 0x467a48 in _start (/ramdisk/php-fuzz/phuzzer/php-70/sapi/cli/php+0x467a48)

0x60200000ab31 is located 0 bytes to the right of 1-byte region [0x60200000ab30,0x60200000ab31)
allocated by thread T0 here:
    #0 0x7fa9ce45d54a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9854a)
    #1 0x98d8ef in php_gd_gdImageSetStyle /home/operac/php-70/ext/gd/libgd/gd.c:2814
    #2 0x94dc9e in zif_imagesetstyle /home/operac/php-70/ext/gd/gd.c:1463
    #3 0x1da38da in ZEND_DO_ICALL_SPEC_HANDLER /home/operac/php-70/Zend/zend_vm_execute.h:586
    #4 0x1b4c335 in execute_ex /home/operac/php-70/Zend/zend_vm_execute.h:414
    #5 0x1df9dc8 in zend_execute /home/operac/php-70/Zend/zend_vm_execute.h:458
    #6 0x194764a in zend_execute_scripts /home/operac/php-70/Zend/zend.c:1427
    #7 0x16b8347 in php_execute_script /home/operac/php-70/main/main.c:2494
    #8 0x1e02126 in do_cli /home/operac/php-70/sapi/cli/php_cli.c:974
    #9 0x467378 in main /home/operac/php-70/sapi/cli/php_cli.c:1344
    #10 0x7fa9cc55782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/operac/php-70/ext/gd/libgd/gd.c:734 php_gd_gdImageSetPixel


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-07-29 22:43 UTC] stas@php.net
-Type: Security +Type: Bug
 [2016-07-30 09:54 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2016-08-02 15:42 UTC] cmb@php.net
-Summary: imagesetstyle function causes read out of bounds +Summary: imagesetstyle() causes OOB read for empty $styles
 [2016-08-02 17:06 UTC] cmb@php.net
Automatic comment on behalf of cmb
Revision: http://git.php.net/?p=php-src.git;a=commit;h=f5622f5c8763fe180310ed7a47b999f160d7750b
Log: Fix #72709: imagesetstyle() causes OOB read for empty $styles
 [2016-08-02 17:06 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 [2016-08-02 17:08 UTC] cmb@php.net
I've filed <https://github.com/libgd/libgd/issues/279> to improve
the behavior of libgd in this regard.
 [2016-10-17 10:10 UTC] bwoebi@php.net
Automatic comment on behalf of cmb
Revision: http://git.php.net/?p=php-src.git;a=commit;h=f5622f5c8763fe180310ed7a47b999f160d7750b
Log: Fix #72709: imagesetstyle() causes OOB read for empty $styles
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Wed Aug 23 23:01:39 2017 UTC