search

tbeu / matio

MATLAB MAT File I/O Library
https://matio.sourceforge.io
BSD 2-Clause "Simplified" License
334 stars 97 forks source link

some memory corruption problems when the library parse the mat file #103

Closed cool-tomato closed 5 years ago

cool-tomato commented 5 years ago

I found several memory corruption problem in the library. More details can be found at here.

PhilipMorrisJones commented 5 years ago

@tbeu, can you comment on this and say if it is a real vulnerability or not and if so when it is likely to be fixed?

There are 13 CVEs related to this

CVE-2019-9038 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is an out-of-bounds read problem with a SEGV in the function ReadNextCell() in mat5.c.
CVE-2019-9037 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a buffer over-read in the function Mat_VarPrint() in mat.c.
CVE-2019-9036 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a heap-based buffer overflow in the function ReadNextFunctionHandle() in mat5.c.
CVE-2019-9035 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a stack-based buffer over-read in the function ReadNextStructField() in mat5.c.
CVE-2019-9034 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a stack-based buffer over-read for a memcpy in the function ReadNextCell() in mat5.c.
CVE-2019-9033 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a stack-based buffer over-read for the "Rank and Dimension" feature in the function ReadNextCell() in mat5.c.
CVE-2019-9032 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is an out-of-bounds write problem causing a SEGV in the function Mat_VarFree() in mat.c.
CVE-2019-9031 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a NULL pointer dereference in the function Mat_VarFree() in mat.c.
CVE-2019-9030 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a stack-based buffer over-read in Mat_VarReadNextInfo5() in mat5.c.
CVE-2019-9029 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is an out-of-bounds read with a SEGV in the function Mat_VarReadNextInfo5() in mat5.c.
CVE-2019-9028 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a stack-based buffer over-read in the function InflateDimensions() in inflate.c when called from ReadNextCell in mat5.c.
CVE-2019-9027 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a heap-based buffer overflow problem in the function ReadNextCell() in mat5.c.
CVE-2019-9026 An issue was discovered in libmatio.a in matio (aka MAT File I/O Library) 1.5.13. There is a heap-based buffer overflow in the function InflateVarName() in inflate.c when called from ReadNextCell in mat5.c.
tbeu commented 5 years ago

Working on them. (That's why I pinned the issue).

tbeu commented 5 years ago

And, it only happens with crafted MAT-files (created by fuzzing).

PhilipMorrisJones commented 5 years ago

@tbeu, thank you. We use this library and appreciate its compactness and usefulness.

Do you know anything about @cool-tomato and why she would fuzz your code?

tbeu commented 5 years ago

Fuzzing is easy, probably fun and gives you some credits. Do not know him/her.

tbeu commented 5 years ago

Resolved by a0539135c9b1ab7613aa7953279da9224da88775 and 2c20d2178017b3eb13ab160cef239648f9915bdb in master branch.

TeoShaw commented 5 years ago

Great to hear that this has been resolved.

On what time scale will we be able to download 1.5.14 and get access to these security improvements?

Thanks,

T.

tbeu commented 5 years ago

Will release v1.5.14 probably tonight.

svillemot commented 5 years ago

As far as I can tell, CVE-2019-9036 (heap-based buffer overflow in the function ReadNextFunctionHandle()) is not yet fixed.

tbeu commented 5 years ago

Hm, can no longer reproduce. Can you give some more details?

svillemot commented 5 years ago

On git HEAD (9f7f96d727d), with Debian 9, configured with ./configure CFLAGS="-fsanitize=address -O2" LDFLAGS="-fsanitize=address":

$ tools/matdump ~/pocs/matio/ReadNextFunctionHandle@mat5.c_1837-26___heap-buffer-overflow                                                
=================================================================
==29570==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000efd0 at pc 0x7f5b9c14f9fe bp 0x7ffca66d34a0 sp 0x7ffca66d3498
WRITE of size 8 at 0x60200000efd0 thread T0
    #0 0x7f5b9c14f9fd in Mat_VarReadNextInfo5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbf9fd)
    #1 0x562dd8c1c9b9 in main (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x59b9)
    #2 0x7f5b9b7f32e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #3 0x562dd8c1d249 in _start (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x6249)

0x60200000efd1 is located 0 bytes to the right of 1-byte region [0x60200000efd0,0x60200000efd1)
allocated by thread T0 here:
    #0 0x7f5b9c42dd28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
    #1 0x7f5b9c14ef7a in Mat_VarReadNextInfo5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbef7a)
    #2 0x7f5b9c1634a7  (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xd34a7)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbf9fd) in Mat_VarReadNextInfo5
Shadow bytes around the buggy address:
  0x0c047fff9da0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9db0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9dc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9dd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9de0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9df0: fa fa fa fa fa fa fa fa fa fa[01]fa fa fa 00 fa
  0x0c047fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9e40: 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
==29570==ABORTING
tbeu commented 5 years ago

Got it.

carnil commented 5 years ago

Should the issue in meanwhile be reopened until as well the last bit fixed?

tbeu commented 5 years ago

@svillemot Can you please check if 539ca4df145748558d79ac978d05857437ba3332 fixes the issue for you. Thanks.

svillemot commented 5 years ago

The overflow seems to be correctly worked around. But then MatIO apparently attempts to allocate an insane amount of memory, I am not sure this is expected:

$ tools/matdump ~/pocs/matio/ReadNextFunctionHandle@mat5.c_1837-26___heap-buffer-overflow
Integer multiplication overflow when calculating number of elements
==14602==WARNING: AddressSanitizer failed to allocate 0x4098900aa0000000 bytes
==14602==AddressSanitizer's allocator is terminating the process instead of returning 0
==14602==If you don't like this behavior set allocator_may_return_null=1
==14602==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/sanitizer_common/sanitizer_allocator.cc:145 "((0)) != (0)" (0x0, 0x0)
    #0 0x7f04a03f5ebd  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xcaebd)
    #1 0x7f04a03fbb13 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xd0b13)
    #2 0x7f04a03f9cd6  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xcecd6)
    #3 0x7f04a0350144  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x25144)
    #4 0x7f04a03ecd05 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d05)
    #5 0x7f04a010d70f in Mat_VarReadNextInfo5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbe70f)
    #6 0x55c2c84699b9 in main (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x59b9)
    #7 0x7f049f7b22e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #8 0x55c2c846a249 in _start (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x6249)
tbeu commented 5 years ago

Thanks for confirmation. One other possibility would be to let SafeMulDims return 0 in case of an overflow. What do you think?

svillemot commented 5 years ago

Indeed it's probably better to have a zero return value from SafeMulDims, which would then be catched and properly handled by the caller.

tbeu commented 5 years ago

@svillemot Could you please give 077cbf9ecf495c15edc8546c1acb0cbabf8b6e51 one more try and verify if it finally resolves this issue? Thanks.

svillemot commented 5 years ago

Thanks. The only thing still detected by ASAN is a 1-byte memory leak. So, security-wise, the issue is fixed.

$ tools/matdump ~/pocs/matio/ReadNextFunctionHandle@mat5.c_1837-26___heap-buffer-overflow
Empty

=================================================================
==14285==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7fb2691fed28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
    #1 0x7fb268f1f1e2 in Mat_VarReadNextInfo5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbf1e2)
    #2 0x7fb268f340af  (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xd40af)

SUMMARY: AddressSanitizer: 1 byte(s) leaked in 1 allocation(s).
tbeu commented 5 years ago

Thanks for confirmation. The memory leak actually is a separate issue discoverd en passant and resolved by b73f135ebb5339eec376b1f25e63ab50d0d43b55.

svillemot commented 5 years ago

I backported those security fixes to MatIO 1.5.13 for Debian (I had to limit myself to minimal changes, since Debian is currently in freeze).

There are test failures on several architectures, all of which are big-endian, in tests 621, 2825 and 2827. Any idea of what's going on?

svillemot commented 5 years ago

The logfiles are accessible at: https://buildd.debian.org/status/package.php?p=libmatio

svillemot commented 5 years ago

And here are the patches that I applied: https://salsa.debian.org/science-team/libmatio/tree/master/debian/patches

Note that avoid-int-mult-overflow.patch is a trimmed-down version of your commit.

svillemot commented 5 years ago

Ok, got it, it's a manifestation of #108.

svillemot commented 5 years ago

Applying adfa218770183cf93f74e7fad5055921ae1f9958 fixes the testsuite regression, but unfortunately it reintroduces CVE-2019-9027 and CVE-2019-9038.

On the current git HEAD, I now get the following:

$ tools/matdump ~/pocs/matio/inflate___heap-buffer-overflow-02
=================================================================
==17501==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000a739 at pc 0x7fb9549d7f7f bp 0x7ffcfb664400 sp 0x7ffcfb663bb0
READ of size 64 at 0x60200000a739 thread T0
    #0 0x7fb9549d7f7e  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x5cf7e)
    #1 0x7fb953f14238 in inflate (/lib/x86_64-linux-gnu/libz.so.1+0xa238)
    #2 0x7fb954699abb in InflateData (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xdabb)
    #3 0x7fb9546c3184 in ReadCompressedCharData (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0x37184)
    #4 0x7fb9547436fd in Mat_VarRead5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xb76fd)
    #5 0x7fb95474804f in ReadNextCell (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbc04f)
    #6 0x7fb95474e53a in Mat_VarReadNextInfo5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xc253a)
    #7 0x55625cfea9c9 in main (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x59c9)
    #8 0x7fb9538872e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #9 0x55625cfeb259 in _start (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x6259)

0x60200000a739 is located 0 bytes to the right of 9-byte region [0x60200000a730,0x60200000a739)
allocated by thread T0 here:
    #0 0x7fb954a3ced0 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1ed0)
    #1 0x7fb954740187 in Mat_VarRead5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xb4187)
    #2 0x7fb95476ea4f  (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xe2a4f)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x5cf7e)
Shadow bytes around the buggy address:
  0x0c047fff9490: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff94a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff94b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff94c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff94d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff94e0: fa fa fa fa fa fa 00[01]fa fa 00 00 fa fa 00 01
  0x0c047fff94f0: fa fa 00 00 fa fa 00 fa fa fa 00 fa fa fa 00 00
  0x0c047fff9500: fa fa 00 fa fa fa 00 00 fa fa 00 fa fa fa 00 fa
  0x0c047fff9510: fa fa 01 fa fa fa 00 00 fa fa 00 fa fa fa 00 fa
  0x0c047fff9520: fa fa 00 00 fa fa 00 fa fa fa 00 fa fa fa 00 00
  0x0c047fff9530: fa fa 00 fa fa fa 00 fa fa fa 00 00 fa fa 00 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
==17501==ABORTING
$ tools/matdump ~/pocs/matio/ReadNextCellt@mat5.c_1342-33__out-of-bound-read
InflateData: inflate returned data error
InflateSkip: inflate returned data error
ASAN:DEADLYSIGNAL
=================================================================
==14816==ERROR: AddressSanitizer: SEGV on unknown address 0x000d0d8dcdf7 (pc 0x7f1ba88b5077 bp 0x606000003140 sp 0x7ffdb11b5a70 T0)
    #0 0x7f1ba88b5076 in ReadNextCell (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbc076)
    #1 0x7f1ba88bb53a in Mat_VarReadNextInfo5 (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xc253a)
    #2 0x5581aaa359c9 in main (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x59c9)
    #3 0x7f1ba79f42e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #4 0x5581aaa36259 in _start (/home/sebastien/debian/upstream/matio/tools/.libs/matdump+0x6259)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/sebastien/debian/upstream/matio/src/.libs/libmatio.so.9+0xbc076) in ReadNextCell
==14816==ABORTING

So this issue should be reopened…

tbeu commented 5 years ago

One more iteration loop please: 02625a0e394eeb8bf3ea61641f73de907296a2c4 adds another sanity check.

svillemot commented 5 years ago

It is good now, thanks!