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 overrun: write extra bytes in TNEFFillMapi #47

Closed bingosxs closed 6 years ago

bingosxs commented 7 years ago

We discover a buffer over-write problem in the T::NEFFillMapi functiion. The root cause of this problem is zero-byte allocation problem. in lib/ytnef.c:485 mp->data = calloc(mp->count, sizeof(variableLength)); the value of mp->count can be zero and the write and read operation cause problems in TNEFFillMapi (ytnef.c:508) ,TNEFFillMapi (ytnef.c:517), TNEFFillMapi (ytnef.c:526). The following patch can solve this issue:

--- ../ytnef/lib/ytnef.c    2017-05-14 19:44:47.417979746 +0800
+++ lib/ytnef.c 2017-05-16 08:59:28.088356920 +0800
@@ -479,7 +482,9 @@
         d += 4;
         count = 0;
       }
-      mp->data = calloc(mp->count, sizeof(variableLength));
+      if (mp->count>0){
+      mp->data = calloc(mp->count, sizeof(variableLength)); 
+      }else{ mp->data=NULL;      }
       ALLOCCHECK(mp->data);
       vl = mp->data;
     } else {

The tracelog is shown below:

valgrind ./bin/ytnef -v ../../libytnef0/testenv/out/crashes/id\:000015\,sig\:06\,src\:000007\,op\:int16\,pos\:2540\,val\:+1
==12741== Memcheck, a memory error detector
==12741== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==12741== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==12741== Command: ./bin/ytnef -v ../../libytnef0/testenv/out/crashes/id:000015,sig:06,src:000007,op:int16,pos:2540,val:+1
==12741== 
Attempting to parse ../../libytnef0/testenv/out/crashes/id:000015,sig:06,src:000007,op:int16,pos:2540,val:+1...
==12741== Invalid write of size 4
==12741==    at 0x4E3F89A: TNEFFillMapi (ytnef.c:504)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741==  Address 0x542a488 is 8 bytes after a block of size 0 alloc'd
==12741==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12741==    by 0x4E3F213: TNEFFillMapi (ytnef.c:482)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741== 
==12741== Invalid write of size 8
==12741==    at 0x4E3FA0A: TNEFFillMapi (ytnef.c:513)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741==  Address 0x542a480 is 0 bytes after a block of size 0 alloc'd
==12741==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12741==    by 0x4E3F213: TNEFFillMapi (ytnef.c:482)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741== 
==12741== Invalid read of size 4
==12741==    at 0x4E3FA50: TNEFFillMapi (ytnef.c:515)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741==  Address 0x542a488 is 8 bytes after a block of size 0 alloc'd
==12741==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12741==    by 0x4E3F213: TNEFFillMapi (ytnef.c:482)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741== 
==12741== Invalid read of size 4
==12741==    at 0x4E3FAA1: TNEFFillMapi (ytnef.c:522)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741==  Address 0x542a488 is 8 bytes after a block of size 0 alloc'd
==12741==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12741==    by 0x4E3F213: TNEFFillMapi (ytnef.c:482)
==12741==    by 0x4E3D582: TNEFMapiProperties (ytnef.c:396)
==12741==    by 0x4E46C49: TNEFParse (ytnef.c:1184)
==12741==    by 0x4E45C85: TNEFParseFile (ytnef.c:1042)
==12741==    by 0x4017F8: main (main.c:125)
==12741== 
Corrupted file detected at ytnef.c : 509
ERROR Parsing MAPI block
==12741== 
==12741== HEAP SUMMARY:
==12741==     in use at exit: 9 bytes in 1 blocks
==12741==   total heap usage: 167 allocs, 166 frees, 18,316 bytes allocated
==12741== 
==12741== LEAK SUMMARY:
==12741==    definitely lost: 9 bytes in 1 blocks
==12741==    indirectly lost: 0 bytes in 0 blocks
==12741==      possibly lost: 0 bytes in 0 blocks
==12741==    still reachable: 0 bytes in 0 blocks
==12741==         suppressed: 0 bytes in 0 blocks
==12741== Rerun with --leak-check=full to see details of leaked memory
==12741== 
==12741== For counts of detected and suppressed errors, rerun with: -v
==12741== ERROR SUMMARY: 5 errors from 4 contexts (suppressed: 0 from 0)

The testcase can be downloaded here testcase

bingosxs commented 7 years ago

The CVE ID: CVE-2017-9146

kirotawa commented 6 years ago

Is there already any fix for this CVE/Bug ?