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

[bingosxs]

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]

this is CVE-2017-9058 ...

[kirotawa]

Is there any fix for this CVE/bug already?

[ohwgiles]

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

[ohwgiles]

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]

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