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

go to bug id or search bugs for

Sec Bug #76130 Heap Buffer Overflow (READ: 1786) in exif_iif_add_value
Submitted: 2018-03-21 18:31 UTC Modified: 2018-04-29 20:48 UTC
From: cyoung at tripwire dot com Assigned: stas (profile)
Status: Closed Package: EXIF related
PHP Version: 5.6 + OS: Linux
Private report: No CVE-ID: 2018-10549
View Developer Edit
 [2018-03-21 18:31 UTC] cyoung at tripwire dot com 
Description:
------------
The exif_read_data() function is prone to an out of bounds read while processing crafted JPG data.  This was discovered using AFL.



Test script:
---------------
USE_ZEND_ALLOC=0 php -r 'exif_read_data("data://text/plain;base64,/9gwABAwMDAwMDAwMDAwMDAwMOENMEV4aWYAAElJKgAIAAAAMAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAFAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADAGAAAwMDAwMAAAADACAAAwMDAwMAAAADAGAAB8kjAwMAAAADAGAAAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw");'

Actual result:
--------------
When running the test script with an ASAN enabled PHP interpreter with USE_ZEND_ALLOC=0, the following ASAN report/backtrace is generated:
==10816==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f000001bb1 at pc 0x00000045fe6a bp 0x7ffcf22faf00 sp 0x7ffcf22fa6b0
READ of size 1786 at 0x61f000001bb1 thread T0
    #0 0x45fe69 in __interceptor_strlen.part.31 /home/cyoung/llvm/llvm-4.0.0.src/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:284
    #1 0x103e09f in exif_iif_add_value /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:2115:15
    #2 0x103e09f in exif_iif_add_tag /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:2199
    #3 0x1047bdf in exif_process_IFD_TAG /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:3543:2
    #4 0x1043550 in exif_process_IFD_in_JPEG /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:3576:8
    #5 0x1039a1f in exif_process_TIFF_in_JPEG /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:3665:2
    #6 0x1039a1f in exif_process_APP1 /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:3690
    #7 0x1039a1f in exif_scan_JPEG_header /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:3835
    #8 0x1039a1f in exif_scan_FILE_header /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4224
    #9 0x1039a1f in exif_read_from_impl /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4365
    #10 0x1039a1f in exif_read_from_stream /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4382
    #11 0x10302d9 in exif_read_from_file /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4409:8
    #12 0x10302d9 in zif_exif_read_data /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4484
    #13 0x1d7899d in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_vm_execute.h:573:2
    #14 0x1ae22dd in execute_ex /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_vm_execute.h:59723:7
    #15 0x1ae2b92 in zend_execute /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_vm_execute.h:63760:2
    #16 0x18e2be0 in zend_eval_stringl /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_execute_API.c:1082:4
    #17 0x18e36da in zend_eval_stringl_ex /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_execute_API.c:1123:11
    #18 0x18e36da in zend_eval_string_ex /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_execute_API.c:1134
    #19 0x1eda9f9 in do_cli /home/cyoung/php/7.2.3/php-src-php-7.2.3/sapi/cli/php_cli.c:1042:8
    #20 0x1ed78be in main /home/cyoung/php/7.2.3/php-src-php-7.2.3/sapi/cli/php_cli.c:1404:18
    #21 0x7f6b51b8e82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #22 0x43b308 in _start (/home/cyoung/php/7.2.3/php-src-php-7.2.3/sapi/cli/php+0x43b308)

0x61f000001bb1 is located 0 bytes to the right of 3377-byte region [0x61f000000e80,0x61f000001bb1)
allocated by thread T0 here:
    #0 0x4f3988 in __interceptor_malloc /home/cyoung/llvm/llvm-4.0.0.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:66
    #1 0x180e0dc in __zend_malloc /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_alloc.c:2829:14
    #2 0x10302d9 in exif_read_from_file /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4409:8
    #3 0x10302d9 in zif_exif_read_data /home/cyoung/php/7.2.3/php-src-php-7.2.3/ext/exif/exif.c:4484
    #4 0x1d7899d in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_vm_execute.h:573:2
    #5 0x1ae22dd in execute_ex /home/cyoung/php/7.2.3/php-src-php-7.2.3/Zend/zend_vm_execute.h:59723:7

Patches

fix-71630.patch (last revision 2018-03-27 17:02 UTC by cmb@php.net)
zero-data.patch (last revision 2018-03-22 14:36 UTC by cmb@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-03-21 18:41 UTC] cyoung at tripwire dot com 
I have placed a test case on my web server: https://secur3.us/php_bugs/exif-oob.jpg

Just in case there are problems using the base64 encoded image stream, you can download the file for testing.
 [2018-03-22 14:36 UTC] cmb@php.net
-Status: Open +Status: Verified
 [2018-03-22 14:36 UTC] cmb@php.net 
I can confirm the issue for PHP-7.0 up to master.  If have not
checked PHP-5.6, but it appears to be affected as well.  For
master (@e791e93) valgrind reports:

==880== Conditional jump or move depends on uninitialised value(s)
==880==    at 0x4C2EDB8: strlen (vg_replace_strmem.c:454)
==880==    by 0x2E242C: exif_iif_add_value (exif.c:2093)
==880==    by 0x2E2765: exif_iif_add_tag (exif.c:2177)
==880==    by 0x2E5D0A: exif_process_IFD_TAG (exif.c:3519)
==880==    by 0x2E5ED5: exif_process_IFD_in_JPEG (exif.c:3552)
==880==    by 0x2E624D: exif_process_TIFF_in_JPEG (exif.c:3641)
==880==    by 0x2E634F: exif_process_APP1 (exif.c:3666)
==880==    by 0x2E6911: exif_scan_JPEG_header (exif.c:3811)
==880==    by 0x2E7A67: exif_scan_FILE_header (exif.c:4200)
==880==    by 0x2E858E: exif_read_from_impl (exif.c:4341)
==880==    by 0x2E85EF: exif_read_from_stream (exif.c:4358)
==880==    by 0x2E86BC: exif_read_from_file (exif.c:4385)
==880==  Uninitialised value was created by a heap allocation
==880==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==880==    by 0x4A92E0: __zend_malloc (zend_alloc.c:2886)
==880==    by 0x4A81DE: _emalloc (zend_alloc.c:2480)
==880==    by 0x4A8767: _safe_emalloc (zend_alloc.c:2542)
==880==    by 0x2E1F4E: exif_file_sections_add (exif.c:1992)
==880==    by 0x2E66AD: exif_scan_JPEG_header (exif.c:3765)
==880==    by 0x2E7A67: exif_scan_FILE_header (exif.c:4200)
==880==    by 0x2E858E: exif_read_from_impl (exif.c:4341)
==880==    by 0x2E85EF: exif_read_from_stream (exif.c:4358)
==880==    by 0x2E86BC: exif_read_from_file (exif.c:4385)
==880==    by 0x2E8E1F: zif_exif_read_data (exif.c:4460)
==880==    by 0x5545D1: ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execut
e.h:641)

Apparently, we should replace the unnecessary (there can't be an
integer overflow in this case) safe_emalloc() with ecalloc() (see
the attached patch zero-data.patch).

@cyoung: Can you please verify whether the patch would solve the
issue?
 [2018-03-22 14:36 UTC] cmb@php.net 
The following patch has been added/updated:

Patch Name: zero-data.patch
Revision:   1521729404
URL:        https://bugs.php.net/patch-display.php?bug=76130&patch=zero-data.patch&revision=1521729404
 [2018-03-22 15:12 UTC] cyoung at tripwire dot com 
@cmb I can definitely confirm the patch but I am unable to access the file.  (I get an error "You have no access to bug #76130" and I do not see an option to supply my password.) Is there another way to access the patch?
 [2018-03-22 15:12 UTC] cyoung at tripwire dot com 
Related To: Bug #76130
 [2018-03-22 15:21 UTC] cmb@php.net 
> Is there another way to access the patch?

I'm pasting inline:

 ext/exif/exif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/exif/exif.c b/ext/exif/exif.c
index d1155de93f..312399ed52 100644
--- a/ext/exif/exif.c
+++ b/ext/exif/exif.c
@@ -1989,7 +1989,7 @@ static int exif_file_sections_add(image_info_type *ImageInfo, int type, size_t s
 	if (!size) {
 		data = NULL;
 	} else if (data == NULL) {
-		data = safe_emalloc(size, 1, 0);
+		data = ecalloc(size, 1);
 	}
 	ImageInfo->file.list[count].type = type;
 	ImageInfo->file.list[count].data = data;
 [2018-03-22 15:40 UTC] cyoung at tripwire dot com 
The patch does resolve most of my test cases but there are a few which are still hitting an OOB read.  Check out https://secur3.us/php_bugs/exif-oob2.jpg 

Base64 of the input is as follows:
SUkqAAgAAABsAHySfwAUAAAAHgEAAEoBAgAUAAD/mgYAAGmHAhQAAAAWCwAAAKUCwgAUAAEAWgUA
AgICAgICAgICCJICABQAAABCCQACAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICCgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgKA////AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIC
AgICAgICAgICAgICAgICAgICAgICAgI8P3BocCBwaHBpbmYFKCk7ID8+ADw/cGhwIHBocGluZm8o
KTsgPz4APD9waAH1AQEBAQEBAQEBATw/cGhwIHBocGluZm8oKQEBAQEBAQEBAQEBAQEBAQEBAQEB
AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAf/CABEIAWQCgAMBEQBCEAEAEQH/xAAfAAEA
AAUFAQAAAAAAAAAAAAAABwALpAIAFAAAAHIG2gAIAQEAAAAA
 [2018-03-22 16:55 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2018-03-22 16:55 UTC] cmb@php.net 
> The patch does resolve most of my test cases […]

Thanks for the quick confirmation!

> but there are a few which are still hitting an OOB read.

I can confirm that.  Will investigate ASAP.
 [2018-03-22 17:09 UTC] stas@php.net 
I usually use private gists to post patches, this way you can be sure they are downloadable. 

@cmb: could you explain the patch? Do we have reference to uninitialized memory there? If so, shouldn't we fix whatever code is accessing uninitialized memory too?
 [2018-03-22 17:10 UTC] stas@php.net
-CVE-ID: +CVE-ID: needed
 [2018-03-23 12:54 UTC] cmb@php.net
-Assigned To: cmb +Assigned To: kalle
 [2018-03-23 12:54 UTC] cmb@php.net 
> https://secur3.us/php_bugs/exif-oob2.jpg

This issue seems to be not directly related to the other one.  The
problem is that the first IFD entry of the image (which is
actually a TIFF image), which is a MakerNote, specifies its format
as x7F which is illegal, so the code assumes it's BYTE.  Since the
length is 20, we must not read more than 20 bytes.  However, we're
calculating strlen(value)[1], but there is no NUL byte within the
first 20 bytes, which causes an OOB read in this case. The
solution would be to replace the line with:

  length = (int) php_strnlen(value, length);

> could you explain the patch?

Not really.  I've just acted on the valgrind report, which claimed
"Conditional jump or move depends on uninitialised value" pointing
to the respective safe_emalloc().

I think it would be best if Kalle could review the suggested fixes
and potentially related issues.

[1] <https://github.com/php/php-src/blob/PHP-7.2.4/ext/exif/exif.c#L2115>
 [2018-03-23 13:16 UTC] cmb@php.net 
>> https://secur3.us/php_bugs/exif-oob2.jpg
>
> This issue seems to be not directly related to the other one.

Correction.  Actually, it seems that fixing the strlen() only,
also solves the OOB read for the test script provided in the OP,
so there might be no need to zero the memory as suggested by
zero-data.patch.
 [2018-03-23 13:29 UTC] cyoung at tripwire dot com 
I agree.  I had confused these test cases as the same bug because they were both having the OOB read on strlen based on the ASAN output. 

I've confirmed that this fixes the test cases in my queue and now I'll resume fuzzing with the php_strnlen() as further confirmation.
 [2018-03-23 14:45 UTC] cyoung at tripwire dot com 
I have done some fuzzing with the php_strnlen patch both with and without the ecalloc patch.  I'm no longer getting ASAN reports with either build.
 [2018-03-27 17:02 UTC] cmb@php.net 
The following patch has been added/updated:

Patch Name: fix-71630.patch
Revision:   1522170171
URL:        https://bugs.php.net/patch-display.php?bug=76130&patch=fix-71630.patch&revision=1522170171
 [2018-03-27 17:03 UTC] cmb@php.net
-Status: Verified +Status: Analyzed -PHP Version: 7.2.3 +PHP Version: 5.6 + -Assigned To: kalle +Assigned To: stas
 [2018-03-27 17:03 UTC] cmb@php.net 
Thanks for further fuzz testing, @cyoung!

Since the php_strlen() patch obviously fixes the issue,
initializing the memory (zero-data.patch) is not necessary.

Stas, can you please apply fix-76130.patch to PHP-5.6 and merge
upward?
 [2018-03-28 04:45 UTC] stas@php.net 
@cmb since the releases seem to be already tagged, I think I'll apply it to the next one (in a month).
 [2018-03-28 13:30 UTC] cmb@php.net 
@stas That's fine by me.
 [2018-04-16 13:47 UTC] cyoung at tripwire dot com 
Is there a CVE assignment pending or should I go through DWF to get one?
 [2018-04-16 13:53 UTC] kaplan@php.net 
We'll ask for a CVE after the release of the fix (makes the process easier).
 [2018-04-23 03:36 UTC] stas@php.net 
Fix in security repo as b4e4788c4461449b4587e19ef1f474ce938e4980
 [2018-04-24 05:13 UTC] stas@php.net
-Status: Analyzed +Status: Closed
 [2018-04-24 05:13 UTC] stas@php.net 
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.
 [2018-04-29 20:48 UTC] kaplan@php.net
-CVE-ID: needed +CVE-ID: 2018-10549
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 07:01:32 2024 UTC