php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #73549 Use after free when stream is passed to imagepng
Submitted: 2016-11-16 21:51 UTC Modified: 2016-12-01 21:36 UTC
From: marceloje at gmail dot com Assigned: cmb (profile)
Status: Closed Package: GD related
PHP Version: 5.6.28 OS: Linux x86_64
Private report: No CVE-ID: None
View Developer Edit
 [2016-11-16 21:51 UTC] marceloje at gmail dot com 
Description:
------------
When an invalid file is supplied to "imagepng" function, "Use after free" happens while calling destructors.

Test script:
---------------
poc.php

<?php

$img1 = fopen("/dev/zero", "r");
$img2 = imagecreatetruecolor(100, 100);
imagepng($img2, $img1);


Expected result:
----------------
Not crash

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

USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0 php -n poc.php


==20350==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300006e440 at pc 0x000000e40451 bp 0x7fffffffb110 sp 0x7fffffffb100
READ of size 4 at 0x60300006e440 thread T0
    #0 0xe40450 in zval_delref_p /home/operac/build5/php-src/Zend/zend_types.h:827
    #1 0xe40b52 in i_zval_ptr_dtor /home/operac/build5/php-src/Zend/zend_variables.h:57
    #2 0xe41bfa in _zval_ptr_dtor_wrapper /home/operac/build5/php-src/Zend/zend_variables.c:259
    #3 0xe7e521 in _zend_hash_del_el_ex /home/operac/build5/php-src/Zend/zend_hash.c:1026
    #4 0xe7e826 in _zend_hash_del_el /home/operac/build5/php-src/Zend/zend_hash.c:1050
    #5 0xe816fc in zend_hash_graceful_reverse_destroy /home/operac/build5/php-src/Zend/zend_hash.c:1506
    #6 0xe09467 in shutdown_executor /home/operac/build5/php-src/Zend/zend_execute_API.c:277
    #7 0xe46e69 in zend_deactivate /home/operac/build5/php-src/Zend/zend.c:967
    #8 0xd09571 in php_request_shutdown /home/operac/build5/php-src/main/main.c:1833
    #9 0x1059aa1 in do_cli /home/operac/build5/php-src/sapi/cli/php_cli.c:1141
    #10 0x105a82c in main /home/operac/build5/php-src/sapi/cli/php_cli.c:1344
    #11 0x7ffff494f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #12 0x430a88 in _start (/home/operac/build5/bin/php+0x430a88)

0x60300006e440 is located 0 bytes inside of 24-byte region [0x60300006e440,0x60300006e458)
freed by thread T0 here:
    #0 0x7ffff6f022ca in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x982ca)
    #1 0xdc7282 in _efree /home/operac/build5/php-src/Zend/zend_alloc.c:2472
    #2 0xe89a0c in list_entry_destructor /home/operac/build5/php-src/Zend/zend_list.c:189
    #3 0xe7e521 in _zend_hash_del_el_ex /home/operac/build5/php-src/Zend/zend_hash.c:1026
    #4 0xe7f9b9 in zend_hash_index_del /home/operac/build5/php-src/Zend/zend_hash.c:1228
    #5 0xe88c9c in zend_list_free /home/operac/build5/php-src/Zend/zend_list.c:59
    #6 0xe41014 in _zval_dtor_func_for_ptr /home/operac/build5/php-src/Zend/zend_variables.c:115
    #7 0xf13705 in zend_vm_stack_free_args /home/operac/build5/php-src/Zend/zend_execute.h:250
    #8 0xf2798c in ZEND_DO_ICALL_SPEC_HANDLER /home/operac/build5/php-src/Zend/zend_vm_execute.h:596
    #9 0xf26872 in execute_ex /home/operac/build5/php-src/Zend/zend_vm_execute.h:414
    #10 0xf26aed in zend_execute /home/operac/build5/php-src/Zend/zend_vm_execute.h:458
    #11 0xe49c66 in zend_execute_scripts /home/operac/build5/php-src/Zend/zend.c:1427
    #12 0xd0b9d8 in php_execute_script /home/operac/build5/php-src/main/main.c:2494
    #13 0x1058a1c in do_cli /home/operac/build5/php-src/sapi/cli/php_cli.c:974
    #14 0x105a82c in main /home/operac/build5/php-src/sapi/cli/php_cli.c:1344
    #15 0x7ffff494f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

previously allocated by thread T0 here:
    #0 0x7ffff6f02602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0xdc8da8 in __zend_malloc /home/operac/build5/php-src/Zend/zend_alloc.c:2864
    #2 0xdc712d in _emalloc /home/operac/build5/php-src/Zend/zend_alloc.c:2457
    #3 0xe888db in zend_list_insert /home/operac/build5/php-src/Zend/zend_list.c:43
    #4 0xe89000 in zend_register_resource /home/operac/build5/php-src/Zend/zend_list.c:98
    #5 0xd49206 in _php_stream_alloc /home/operac/build5/php-src/main/streams/streams.c:310
    #6 0xd5d957 in _php_stream_fopen_from_fd_int /home/operac/build5/php-src/main/streams/plain_wrapper.c:178
    #7 0xd5dff8 in _php_stream_fopen_from_fd /home/operac/build5/php-src/main/streams/plain_wrapper.c:240
    #8 0xd60f58 in _php_stream_fopen /home/operac/build5/php-src/main/streams/plain_wrapper.c:1010
    #9 0xd6134a in php_plain_files_stream_opener /home/operac/build5/php-src/main/streams/plain_wrapper.c:1066
    #10 0xd524e9 in _php_stream_open_wrapper_ex /home/operac/build5/php-src/main/streams/streams.c:2055
    #11 0xb90edc in php_if_fopen /home/operac/build5/php-src/ext/standard/file.c:870
    #12 0x91a9a7 in phar_fopen /home/operac/build5/php-src/ext/phar/func_interceptors.c:427
    #13 0xf277dc in ZEND_DO_ICALL_SPEC_HANDLER /home/operac/build5/php-src/Zend/zend_vm_execute.h:586
    #14 0xf26872 in execute_ex /home/operac/build5/php-src/Zend/zend_vm_execute.h:414
    #15 0xf26aed in zend_execute /home/operac/build5/php-src/Zend/zend_vm_execute.h:458
    #16 0xe49c66 in zend_execute_scripts /home/operac/build5/php-src/Zend/zend.c:1427
    #17 0xd0b9d8 in php_execute_script /home/operac/build5/php-src/main/main.c:2494
    #18 0x1058a1c in do_cli /home/operac/build5/php-src/sapi/cli/php_cli.c:974
    #19 0x105a82c in main /home/operac/build5/php-src/sapi/cli/php_cli.c:1344
    #20 0x7ffff494f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-use-after-free /home/operac/build5/php-src/Zend/zend_types.h:827 zval_delref_p

Patches

fix-73549 (last revision 2016-11-17 13:00 UTC by cmb@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-11-16 22:00 UTC] stas@php.net
-Type: Security +Type: Bug
 [2016-11-17 12:17 UTC] cmb@php.net
-Package: *General Issues +Package: GD related -Assigned To: +Assigned To: cmb
 [2016-11-17 12:59 UTC] cmb@php.net
-Summary: Use after free through invalid file. +Summary: Use after free when stream is passed to imagepng -Status: Assigned +Status: Analyzed
 [2016-11-17 12:59 UTC] cmb@php.net 
This issue is not particularly related to "invalid files", but
rather happens *always* when a stream is passed to one of the
image output functions. As such, it might be regarded medium
severity.

Alternative test script:

    <?php
    $stream = fopen(__DIR__ . DIRECTORY_SEPARATOR . '73549.png', 'w');
    $im = imagecreatetruecolor(100, 100);
    var_dump(imagepng($im, $stream));
    var_dump($stream);

This script shows, that imagepng() closes the supplied stream,
what should, of course, not happen if a stream is supplied, but
only if a filename is given (in which case the stream would have
been opened by _php_image_output_ctx()).

The attached patch (PHP-5.6) is supposed to fix the issue.
 [2016-11-17 12:59 UTC] cmb@php.net
-Type: Bug +Type: Security -Private report: No +Private report: Yes
 [2016-11-17 13:00 UTC] cmb@php.net 
The following patch has been added/updated:

Patch Name: fix-73549
Revision:   1479387619
URL:        https://bugs.php.net/patch-display.php?bug=73549&patch=fix-73549&revision=1479387619
 [2016-11-26 22:54 UTC] stas@php.net 
For me the test does not work:

002+ resource(5) of type (Unknown)
002- resource(%d) of type (stream)

Could you please check?
 [2016-11-27 11:06 UTC] cmb@php.net 
I've just applied the patch against current PHP-5.6 head, and the
test succeeds. The test failure you're getting is expected
*without* the patch.

Note that there's a build flaw in ext/gd where changes to gd_ctx.c
(and other source files) don't trigger re-compilation of gd.(l)o.
In other words, if you already have built PHP-5.6, apply the patch
and do only `make`, the test failure is to be expected.
 [2016-11-27 22:49 UTC] stas@php.net 
My bad, it works.
 [2016-11-27 22:54 UTC] stas@php.net
-Type: Security +Type: Bug -PHP Version: 7.0.13 +PHP Version: 5.6.28
 [2016-11-30 23:13 UTC] davey@php.net 
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=5049ef2f1c496c4964cd147e185c1f765ab0347b
Log: Fix #73549: Use after free when stream is passed to imagepng
 [2016-11-30 23:13 UTC] davey@php.net
-Status: Analyzed +Status: Closed
 [2016-11-30 23:43 UTC] yohgaki@php.net 
It's static function. It does not matter much, but isn't

_php_image_stream_ctxfree_and_close()
or
_php_image_stream_ctxfree_close

are better than

_php_image_stream_ctxfreeandclose()
 [2016-12-01 11:42 UTC] cmb@php.net 
Actually, I would have chosen `_php_image_stream_ctx_free` and `_php_image_stream_ctx_free_and_close`, but as there already was `_php_image_stream_ctxfree` I tried to be consistent.
 [2016-12-01 21:36 UTC] yohgaki@php.net 
Sounds good!
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Mon Dec 30 14:01:28 2024 UTC