search

Yeraze / ytnef

Yeraze's TNEF Stream Reader - for winmail.dat files
GNU General Public License v2.0
32 stars 22 forks source link

heap-buffer-overflow vulnerabilty caused by wrong boundary checking in SIZECHECK #45

Closed bingosxs closed 6 years ago

bingosxs commented 7 years ago

heap-buffer-overflow vulnerabilty: 

#lib/ytnef.c +458:
              mp->propnames[length - 1].data[j] = d[j * 2];

The cause is an incorrect array boundary checking. The proposed patch : use>= instead of >

lib/ytnef.c:60 
-- #define SIZECHECK(x) { if ((((char *)d - (char *)data) + x) > size) {  printf("Corrupted file detected at %s : %i\n", __FILE__, __LINE__); return(-1); } }
++ #define SIZECHECK(x) { if ((((char *)d - (char *)data) + x) >= size) {  printf("Corrupted file detected at %s : %i\n", __FILE__, __LINE__); return(-1); } }

the testcase download link: testcase

The trace log is:

ytnef/.libs/ytnef -v ../libytnef0/testenv/out/crashes/id\:000000\,sig\:11\,src\:000002\,op\:int16\,pos\:7378\,val\:+1 --main-stacksize=flag Attempting to parse ../libytnef0/testenv/out/crashes/id:000000,sig:11,src:000002,op:int16,pos:7378,val:+1...

==7554==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62400000dc4c at pc 0x7fc34a4d3143 bp 0x7fff84db23b0 sp 0x7fff84db23a8 READ of size 1 at 0x62400000dc4c thread T0

0 0x7fc34a4d3142 in TNEFFillMapi /home/canicula/afl/test/ytnef.0/lib/ytnef.c:458:51

#1 0x7fc34a4ccef2 in TNEFMapiProperties /home/canicula/afl/test/ytnef.0/lib/ytnef.c:396:7
#2 0x7fc34a4dde07 in TNEFParse /home/canicula/afl/test/ytnef.0/lib/ytnef.c:1184:15
#3 0x7fc34a4dc6f9 in TNEFParseFile /home/canicula/afl/test/ytnef.0/lib/ytnef.c:1042:10
#4 0x4ea71b in main /home/canicula/afl/test/ytnef.0/ytnef/main.c:125:9
#5 0x7fc3495d482f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291
#6 0x418bd8 in _start (/data/canicula/afl/test/ytnef.0/ytnef/.libs/ytnef+0x418bd8)

0x62400000dc4c is located 0 bytes to the right of 6988-byte region [0x62400000c100,0x62400000dc4c) allocated by thread T0 here:

0 0x4b8e90 in calloc (/data/canicula/afl/test/ytnef.0/ytnef/.libs/ytnef+0x4b8e90)

#1 0x7fc34a4dd21e in TNEFParse /home/canicula/afl/test/ytnef.0/lib/ytnef.c:1154:12
#2 0x7fc34a4dc6f9 in TNEFParseFile /home/canicula/afl/test/ytnef.0/lib/ytnef.c:1042:10
#3 0x4ea71b in main /home/canicula/afl/test/ytnef.0/ytnef/main.c:125:9
#4 0x7fc3495d482f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/canicula/afl/test/ytnef.0/lib/ytnef.c:458:51 in TNEFFillMapi Shadow bytes around the buggy address: 0x0c487fff9b30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c487fff9b80: 00 00 00 00 00 00 00 00 00[04]fa fa fa fa fa fa 0x0c487fff9b90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff9ba0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff9bb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff9bc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff9bd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==7554==ABORTING

alteholz commented 7 years ago

this is CVE-2017-9058 ...

kirotawa commented 7 years ago

Is there any fix for this CVE/bug already?

ohwgiles commented 6 years ago

The proposed patch makes libytnef unable to extract bookmark.htm from the example tnef file attached to this issue. Is it really invalid?

ohwgiles commented 6 years ago

I think the patch suggested in the CVE report is wrong, despite it being picked up by Debian and by Arch.

My suggested fix is in #58. Note that #54 should also be applied or this test case will get stuck in TNEFPrint

Yeraze commented 6 years ago

Released in https://github.com/Yeraze/ytnef/releases/tag/v1.9.3