Bug #72709 [Asn->Csd]: imagesetstyle() causes OOB read for empty $styles

Edit report at https://bugs.php.net/bug.php?id=72709&edit=1 ID: 72709 Updated by: cmb@php.net Reported by: marceloje at gmail dot com Summary: imagesetstyle() causes OOB read for empty $styles -Status: Assigned +Status: Closed Type: Bug Package: GD related Operating System: Linux x86_64 PHP Version: 7.0.9 Assigned To: cmb Block user comment: N Private report: N New Comment: 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 Previous Comments: ------------------------------------------------------------------------ [2016-07-29 22:13:16] 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 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=72709&edit=1