Bug #71534 [Csd]: Type confusion in exif_read_data() leading to heap overflow in debug mode

Edit report at https://bugs.php.net/bug.php?id=71534&edit=1 ID: 71534 Updated by: kalle@php.net Reported by: hlt99 at blinkenshell dot org Summary: Type confusion in exif_read_data() leading to heap overflow in debug mode Status: Closed Type: Bug Package: EXIF related Operating System: Arch Linux (64-bit) PHP Version: 7.0.3 Assigned To: kalle Block user comment: N Private report: N New Comment: 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! Previous Comments: ------------------------------------------------------------------------ [2016-08-07 03:41:20] 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-06 07:40:53] 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-05 08:17:07] 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 07:23:32] 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 06:15:09] 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! ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=71534 -- Edit this bug report at https://bugs.php.net/bug.php?id=71534&edit=1