New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gdCtxPrintf vsnprintf return value not checked - leaks stack memory #211

Closed
fmunozs opened this Issue May 13, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@fmunozs

fmunozs commented May 13, 2016

Description:

See reproduction test case, length from the failed vsnprintf attempt [1] to copy more than 8000 chars on a 4096 buffer, vsnprintf [2] "a return value of size or more means that the output was truncated", however libgd returns this length as is and PHP prints more information from memory than it should.

Libgd isn't checking the vsnprintf return value and PHP 5.5 will print the length specified, leaking memory data.

/* {{{ gdCtxPrintf */
static void gdCtxPrintf(gdIOCtx * out, const char *format, ...)
{
    char buf[4096];
    int len;
    va_list args;

    va_start(args, format);
    len = vsnprintf(buf, sizeof(buf)-1, format, args); 
     // if len > 4096 data was truncated
     // but libgd returns this value as is 
    va_end(args);
    out->putBuf(out, buf, len);
}

This was reported to PHP

https://bugs.php.net/bug.php?id=72115

Compile PHP 5.5 with ASAN.

BP on https://github.com/php/php-src/blob/PHP-5.5/ext/gd/gd_ctx.c#L39

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /home/user/php/php-55/sapi/cli/php -n -dextension=/home/user/php/php-55/modules/gd.so /home/user/img.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".

Breakpoint 1, _php_image_output_putbuf (ctx=0xb46ddf38, buf=0xbfffa69c, l=8017) at /home/user/php/php-55/ext/gd/gd_ctx.c:39
39              return php_write((void *)buf, l TSRMLS_CC);
(gdb) print l
$7 = 8017

[1] https://github.com/libgd/libgd/blob/master/src/gd_xbm.c#L188
[2] http://linux.die.net/man/3/vsnprintf /

Test script:

<?php
$var1=imagecreatetruecolor ( 2 , 2);
$var2=str_repeat("ABCD", 1030);
$var3=0;
imagexbm($var1, $var2, $var3);

Actual result:

#define ABCDAB... <random stuff from memory> ...
=================================================================                                                                                                              
==3897==ERROR: AddressSanitizer: stack-buffer-underflow on address 0xbfffb750 at pc 0xb7aa6dbd bp 0xbfffa408 sp 0xbfff9fdc                                                                                 
READ of size 8017 at 0xbfffb750 thread T0                                                                                                                                                                  
    #0 0xb7aa6dbc  (/usr/lib/i386-linux-gnu/libasan.so.2+0x3ddbc)                                                                                                                                          
    #1 0x99388cf in sapi_cli_single_write /home/user/php/php-55/sapi/cli/php_cli.c:273                                                                                                                     
    #2 0x9938a01 in sapi_cli_ub_write /home/user/php/php-55/sapi/cli/php_cli.c:308                                                                                                                         
    #3 0x920f82f in php_output_op /home/user/php/php-55/main/output.c:1094                                                                                                                                 
    #4 0x920f82f in php_output_write /home/user/php/php-55/main/output.c:270       
    #5 0x919debb in php_write /home/user/php/php-55/main/main.c:671                                                                                                                                        
    #6 0xb44b1a3b in _php_image_output_putbuf /home/user/php/php-55/ext/gd/gd_ctx.c:39                                                                                                                     
    #7 0xb44638ba  (/usr/lib/i386-linux-gnu/libgd.so.3+0x318ba)                                                                                                                                            
    #8 0xb4463ddf in gdImageXbmCtx (/usr/lib/i386-linux-gnu/libgd.so.3+0x31ddf)                                                                                                                            
    #9 0xb44e0a2e in _php_image_output_ctx /home/user/php/php-55/ext/gd/gd_ctx.c:187                                                                                                                           
    #10 0xb44e0a2e in zif_imagexbm /home/user/php/php-55/ext/gd/gd.c:2702
    #11 0x992dc59 in zend_do_fcall_common_helper_SPEC /home/user/php/php-55/Zend/zend_vm_execute.h:550
    #12 0x96854cf in execute_ex /home/user/php/php-55/Zend/zend_vm_execute.h:363
    #13 0x99225d6 in zend_execute /home/user/php/php-55/Zend/zend_vm_execute.h:388
    #14 0x9472ed1 in zend_execute_scripts /home/user/php/php-55/Zend/zend.c:1327
    #15 0x91a7e7c in php_execute_script /home/user/php/php-55/main/main.c:2525
    #16 0x99379c4 in do_cli /home/user/php/php-55/sapi/cli/php_cli.c:994
    #17 0x808a06c in main /home/user/php/php-55/sapi/cli/php_cli.c:1378
    #18 0xb764c636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
    #19 0x808a78a  (/home/user/php/php-55/sapi/cli/php+0x808a78a)

Address 0xbfffb750 is located in stack of thread T0 at offset 0 in frame
    #0 0xb44dfe0f in zif_imagexbm /home/user/php/php-55/ext/gd/gd.c:2701

@vapier vapier added the bug label May 13, 2016

@vapier

This comment has been minimized.

Show comment
Hide comment
@vapier

vapier May 13, 2016

Member

we probably should extend the IO layer to accept a va_list rather than go through a local buffer. then we can just pass it down directly w/out any stack allocation.

Member

vapier commented May 13, 2016

we probably should extend the IO layer to accept a va_list rather than go through a local buffer. then we can just pass it down directly w/out any stack allocation.

@vapier vapier changed the title from gdCtxPrintf vsnprintf return value not checked - leaks memory in PHP 5.5 to gdCtxPrintf vsnprintf return value not checked - leaks stack memory May 14, 2016

@vapier vapier closed this in 4dc1a2d May 14, 2016

@fmunozs

This comment has been minimized.

Show comment
Hide comment
@fmunozs

fmunozs May 27, 2016

Hello, will this get a CVE assigned? Thanks.

fmunozs commented May 27, 2016

Hello, will this get a CVE assigned? Thanks.

@vapier

This comment has been minimized.

Show comment
Hide comment
@vapier

vapier May 27, 2016

Member

i don't care much about CVE's. just upgrade to the latest version and be done.

Member

vapier commented May 27, 2016

i don't care much about CVE's. just upgrade to the latest version and be done.

@kaplanlior

This comment has been minimized.

Show comment
Hide comment
@kaplanlior

kaplanlior commented Jul 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment