php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71534 Type confusion in exif_read_data() leading to heap overflow in debug mode
Submitted: 2016-02-05 11:05 UTC Modified: 2016-08-07 12:51 UTC
From: hlt99 at blinkenshell dot org Assigned: kalle (profile)
Status: Closed Package: EXIF related
PHP Version: 7.0.3 OS: Arch Linux (64-bit)
Private report: No CVE-ID: None
 [2016-02-05 11:05 UTC] hlt99 at blinkenshell dot org
Description:
------------
# Build instructions

    ../configure --enable-debug --enable-exif && make

# Description

When exif_read_data() is used to extract an embedded thumbnail from an invalid
TIFF image a signed integer may be used to set the ImageInfo->Thumbnail.size
field. The bug can be triggered by manipulating an IFD entry describing the
embedded thumbnails size. When using a format tag (offset 0x24 in poc.tiff)
specifying a signed integer type for the (size) value (offset 0x2a in poc.tiff)
ImageInfo->Thumbnail.size can be set to very large values.

    (PHP-7.0.2) exif.c:
    1275   static size_t exif_convert_any_to_int(void *value, int format, ...) {
    [...]
    1280    switch(format) {
    1281        case TAG_FMT_SBYTE:     return *(signed char *)value;
    
    2787   static int exif_process_IFD_TAG(/*...*/)
    [...]
    2934    ImageInfo->Thumbnail.size = exif_convert_any_to_int(value_ptr, format, ImageInfo->motorola_intel);

Later heap memory is allocated using ImageInfo->Thumbnail.size as an argument
of unsigned type to safe_emalloc():

    (PHP-7.0.2) exif.c:
    3509   static int exif_process_IFD_in_TIFF(...) {
    [...]
    3677        ImageInfo->Thumbnail.data = safe_emalloc(ImageInfo->Thumbnail.size, 1, 0);
    3678        php_stream_seek(ImageInfo->infile, ImageInfo->Thumbnail.offset, SEEK_SET);
    3679        fgot = php_stream_read(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);

With ZEND_DEBUG disabled PHP simply exits with a fatal out of memory error.
However if debugging is enabled the call to safe_emalloc() succeeds and returns
a valid pointer pointing to Zend-internal heap memory of a small size. [1]
Thus heap memory (including heap management data) gets overwritten with
contents from the corrupted TIFF image. This allows for remote code execution
(as demonstrated in the gdb log you find below).

[1] For more info on the zend_mm_alloc_heap() bug refer to #<YET-TO-REPORT>!

# PHP versions known to be affected

        7.0.2 (git)
        7.0.3 (git)

Versions prior to 7.0.2 have not been tested.


# PHP versions known to be not affected

        5.6.18 (git)

Versions prior to 5.6.18 have not been tested.


Test script:
---------------
/*
  exif_read_data.php

  poc.tiff:
  http://hlt99.blinkenshell.org/php/poc.tiff

*/
<?php
    $data = exif_read_data("poc.tiff", NULL, true, true);
?>

Actual result:
--------------
The following gdb log only shows good control of registers when the bug is
causing a segfault in strlen(). There likely are other ways to exploit this.

    $ gdb sapi/cli/php
    [...]
    gdb$ r exif_read_data.php
    [...]
    Program received signal SIGSEGV, Segmentation fault.
    [----------------------------------registers-----------------------------------]
    RAX: 0x4142434445464748 ('HGFEDCBA')    <-- !! completely attacker controlled !!
    RBX: 0x7fffffffbc3f --> 0x0 
    RCX: 0x748 
    RDX: 0x1645c20 --> 0xdf00000000 
    RSI: 0x7fffffffb9f0 --> 0x3000000020 (' ')
    RDI: 0x4142434445464748 ('HGFEDCBA')    <-- !! completely attacker controlled !!
    RBP: 0x0 
    RSP: 0x7fffffffafd8 --> 0xc49c23 (<strx_printv+5043>:	mov    QWORD PTR [rsp+0x100],rax)
    RIP: 0x7ffff35180ca (<strlen+42>:	movdqu xmm12,XMMWORD PTR [rax])
    R8 : 0x7fffffffb920 --> 0x0 
    R9 : 0x7fffffffbc3f --> 0x0 
    R10: 0x0 
    R11: 0x12ec79b ("s(%d) :  Freeing 0x%.8lX (%zu bytes), script=%s\n")
    R12: 0x1655c20 --> 0x42cc 
    R13: 0x1627348 --> 0x1645c20 --> 0xdf00000000 
    R14: 0x7fffffffba40 ("[Thu Feb  4 18:28:13 2016]  Script:  '/home/rc0r/doc/advisories/php7/exif/exif_read_data.php'\n")
    R15: 0x4142434445464748 ('HGFEDCBA')    <-- !! completely attacker controlled !!
    EFLAGS: 0x10293 (CARRY parity ADJUST zero SIGN trap INTERRUPT direction overflow)
    [-------------------------------------code-------------------------------------]
    => 0x7ffff35180ca <strlen+42>:	movdqu xmm12,XMMWORD PTR [rax]
    [------------------------------------------------------------------------------]
    gdb$ bt
    #0  0x00007ffff35180ca in strlen () from /usr/lib/libc.so.6
    #1  0x0000000000c49c23 in format_converter (fmt=<optimized out>, odp=<optimized out>, ap=<optimized out>) at /home/rc0r/tmp/php-src/main/snprintf.c:993
    #2  strx_printv (ccp=0x7fffffffb9ec, buf=<optimized out>, len=0x200, format=<optimized out>, ap=0x7fffffffb9f0) at /home/rc0r/tmp/php-src/main/snprintf.c:1248
    #3  0x0000000000c47a08 in ap_php_snprintf (buf=0x7fffffffb9f0 " ", len=0x1645c20, format=0x748 <error: Cannot access memory at address 0x748>)
        at /home/rc0r/tmp/php-src/main/snprintf.c:1293
    #4  0x0000000000c44f6b in php_message_handler_for_zend (message=<optimized out>, data=0x7fffffffcd88) at /home/rc0r/tmp/php-src/main/main.c:1431
    #5  0x0000000000d33b66 in zend_message_dispatcher (message=0x4142434445464748, data=0x7fffffffb9f0) at /home/rc0r/tmp/php-src/Zend/zend.c:998
    #6  0x0000000000cce01c in zend_mm_check_leaks (heap=<optimized out>) at /home/rc0r/tmp/php-src/Zend/zend_alloc.c:2121
    #7  zend_mm_shutdown (heap=<optimized out>, full=<optimized out>, silent=<optimized out>) at /home/rc0r/tmp/php-src/Zend/zend_alloc.c:2193
    #8  0x0000000000cd33d1 in shutdown_memory_manager (silent=0x1645c20, full_shutdown=0xffffb9f0) at /home/rc0r/tmp/php-src/Zend/zend_alloc.c:2629
    #9  0x0000000000c42584 in php_request_shutdown (dummy=<optimized out>) at /home/rc0r/tmp/php-src/main/main.c:1833
    #10 0x0000000000e99fa5 in do_cli (argc=<optimized out>, argv=<optimized out>) at /home/rc0r/tmp/php-src/sapi/cli/php_cli.c:1142
    #11 0x0000000000e97b82 in main (argc=0x2, argv=0x1668a10) at /home/rc0r/tmp/php-src/sapi/cli/php_cli.c:1350
    #12 0x00007ffff34b7610 in __libc_start_main () from /usr/lib/libc.so.6
    #13 0x00000000004697a9 in _start ()

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-02-05 11:16 UTC] hlt99 at blinkenshell dot org
Seems like I can't update the original bug description, so here is the reference to the other bug:


[1] For more info on the zend_mm_alloc_heap() bug refer to #71535!
 [2016-02-08 17:44 UTC] hlt99 at blinkenshell dot org
I made a mistake in the initial report: This particular bug is also present in PHP-5.6.18 although it doesn't surface as a segfault because PHP-5.6.18 is not affected by #71535.

One way to patch this bug would be to rewrite the format tag before setting ImageInfo->Thumbnail.size via exif_convert_any_to_int(). [2]

[2] http://hlt99.blinkenshell.org/php/exif-type-conf.patch
 [2016-02-15 08:23 UTC] stas@php.net
-Type: Security +Type: Bug
 [2016-02-15 08:23 UTC] stas@php.net
Debug mode should never be used in production, thus reclassifying as non-security.
 [2016-02-15 08:24 UTC] stas@php.net
-Summary: Type confusion in exif_read_data() leading to heap overflow +Summary: Type confusion in exif_read_data() leading to heap overflow in debug mode
 [2016-02-17 17:35 UTC] hlt99 at blinkenshell dot org
Be aware that this bug appears harmless just because PHP memory checks prevent it from surfacing. If there ever is a way around these checks (by introducing another bug in the future or whatever) this becomes a real problem.
(I referenced this other bug to show just that. Even if it works in debug-mode PHP only this time.)
 [2016-08-05 06:15 UTC] kalle@php.net
-Status: Open +Status: Feedback
 [2016-08-05 06:15 UTC] kalle@php.net
Hi

Could you re-upload the patch somewhere so I can take a look at it while fixing some other exif related things?

Thanks!
 [2016-08-05 07:23 UTC] hlt99 at blinkenshell dot org
-Status: Feedback +Status: Open
 [2016-08-05 07:23 UTC] hlt99 at blinkenshell dot org
Please take this patch with a grain of salt as it may have unintended side effects! I merely used it to prevent afl-fuzz from running into the crash over and over again.

Now, after you've been warned: patch reuploaded.
 [2016-08-05 08:17 UTC] kalle@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: kalle
 [2016-08-05 08:17 UTC] kalle@php.net
Hi

Thanks a lot for the patch! I can kinda follow were you are going with this and I think I can work with that.

As you note in the patch, returning TAG_FMT_UNDEFINED can have a side effect, since it is used at the end of the big if/else as we pass it to exif_iif_add_tag().

Another thing I was wondering about, is that in your patch, you only cast it to an unsigned int before two instances of exif_convert_any_to_int() (in case the ImageInfo->Thumbnail.data hits either TAG_JPEG_INTERCHANGE_FORMAT_LEN or TAG_STRIP_BYTE_COUNTS), but we still have a few others, is there any reasoning behind this or just an oversight?

Oh and one last favor, the PoC.tiff, could you also re-upload that so I can toy around with it?
 [2016-08-05 08:17 UTC] kalle@php.net
-Status: Assigned +Status: Feedback
 [2016-08-06 07:40 UTC] hlt99 at blinkenshell dot org
-Status: Feedback +Status: Assigned
 [2016-08-06 07:40 UTC] hlt99 at blinkenshell dot org
Sample poc.tiff reuploaded.

From my memories: These were the two code paths I found that crashed PHP. To maintain the smallest possible footprint of my naive patch attempt I added the casts to unsigned types only there. However I'm quite sure I did not check the others. So consider it as an oversight.
 [2016-08-07 03:41 UTC] kalle@php.net
Automatic comment on behalf of kalle
Revision: http://git.php.net/?p=php-src.git;a=commit;h=af56fed73b6a2f07127f48e05aa4837bfc06c42d
Log: Fixed bug #71534 (Type confusion in exif_read_data() leading to heap  overflow in debug mode)
 [2016-08-07 03:41 UTC] kalle@php.net
-Status: Assigned +Status: Closed
 [2016-08-07 03:42 UTC] kalle@php.net
I committed a fix based on your patch to master (7.2.0), could you please check it out and confirm?

Thanks for making PHP even greater!
 [2016-08-07 12:51 UTC] hlt99 at blinkenshell dot org
Looks good to me!

However the commit message is inaccurate/misleading. This type confusion bug is present in both debug mode *and* non-debug mode.
In order to demonstrate possible security implications of this particular bug, I chained it together with another flaw (#71535 [1]) to obtain arbitrary heap memory overwrite. This other bug was only present in debug mode and has already been patched in commit 0b9c87a [2]. Since this february commit the heap issue in debug mode was mitigated, but left this original bug unpatched until now.

I hope this clarifies the now somewhat misleading title of this report.


[1] https://bugs.php.net/bug.php?id=71535
[2] http://git.php.net/?p=php-src.git;a=commit;h=0b9c87a02bacfbf1d1383ad393bda78e5d65570c
 [2016-10-10 11:17 UTC] krakjoe@php.net
Automatic comment on behalf of kalle
Revision: http://git.php.net/?p=php-src.git;a=commit;h=af56fed73b6a2f07127f48e05aa4837bfc06c42d
Log: Fixed bug #71534 (Type confusion in exif_read_data() leading to heap  overflow in debug mode)
 [2017-01-12 09:12 UTC] krakjoe@php.net
Automatic comment on behalf of kalle
Revision: http://git.php.net/?p=php-src.git;a=commit;h=af56fed73b6a2f07127f48e05aa4837bfc06c42d
Log: Fixed bug #71534 (Type confusion in exif_read_data() leading to heap  overflow in debug mode)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 06:01:30 2024 UTC