Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
cool-tomato opened this issue Feb 20, 2019 · 27 comments
Closed

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

cool-tomato opened this issue Feb 20, 2019 · 27 comments
Labels
bug

Comments

@cool-tomato
Copy link

I found several memory corruption problem in the library.
More details can be found at here.
@tbeu tbeu pinned this issue Feb 20, 2019
@tbeu tbeu added the bug label Feb 20, 2019
@tbeu tbeu changed the title some memory corruption problems when the library parse the mat file. some memory corruption problems when the library parse the mat file Feb 22, 2019
@PhilipMorrisJones
Copy link

@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
Copy link
Owner

tbeu commented Mar 7, 2019

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

@tbeu
Copy link
Owner

tbeu commented Mar 7, 2019

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

@PhilipMorrisJones
Copy link

@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
Copy link
Owner

tbeu commented Mar 7, 2019 •
edited
Loading

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

tbeu added a commit that referenced this issue Mar 9, 2019
@tbeu
Fix reading variables from v5 MAT file
5552ebd 
* Check read success
* Check allocation success
* Fix array index out of bounds (if rank > 14)
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
tbeu added a commit that referenced this issue Mar 9, 2019
@tbeu
Fix printing variables from v5 MAT file
0966538 
* Fix illegal memory access
* Fix array index out of bounds
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
tbeu added a commit that referenced this issue Mar 9, 2019
@tbeu
Fix reading variables from v5 MAT file
a053913 
* Check read success
* Check allocation success
* Fix array index out of bounds (if rank > 14)
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
tbeu added a commit that referenced this issue Mar 9, 2019
@tbeu
Fix printing variables from v5 MAT file
2c20d21 
* Fix illegal memory access
* Fix array index out of bounds
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
@tbeu
Copy link
Owner

tbeu commented Mar 9, 2019

Resolved by a053913 and 2c20d21 in master branch.

@tbeu tbeu closed this as completed Mar 9, 2019
@tbeu tbeu unpinned this issue Mar 9, 2019
@TeoShaw
Copy link

TeoShaw commented Mar 11, 2019

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
Copy link
Owner

tbeu commented Mar 11, 2019

Will release v1.5.14 probably tonight.

@svillemot
Copy link
Contributor

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

@tbeu
Copy link
Owner

tbeu commented Mar 12, 2019

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

@svillemot
Copy link
Contributor

svillemot commented Mar 12, 2019 •
edited
Loading

On git HEAD (9f7f96d), 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
Copy link
Owner

tbeu commented Mar 12, 2019

Got it.

@carnil
Copy link

carnil commented Mar 14, 2019

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

@tbeu tbeu reopened this Mar 14, 2019
tbeu added a commit that referenced this issue Mar 15, 2019
@tbeu
Check integer multiplication overflow
69748e2 
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
tbeu added a commit that referenced this issue Mar 15, 2019
@tbeu
Check integer multiplication overflow
539ca4d 
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
@tbeu
Copy link
Owner

tbeu commented Mar 15, 2019

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

@svillemot
Copy link
Contributor

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
Copy link
Owner

tbeu commented Mar 16, 2019

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

@svillemot
Copy link
Contributor

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 added a commit that referenced this issue Mar 19, 2019
@tbeu
Check integer multiplication overflow
1677b49 
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
@tbeu
Copy link
Owner

tbeu commented Mar 19, 2019 •
edited
Loading

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

tbeu added a commit that referenced this issue Mar 19, 2019
@tbeu
Fix integer multiplication overflow
45f0ae8 
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
tbeu added a commit that referenced this issue Mar 19, 2019
@tbeu
Fix integer multiplication overflow
077cbf9 
* As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
@svillemot
Copy link
Contributor

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 added a commit that referenced this issue Mar 20, 2019
@tbeu
Fix memory leak
b73f135 
* As reported by #103
@tbeu
Copy link
Owner

tbeu commented Mar 20, 2019

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

tbeu added a commit that referenced this issue Mar 20, 2019
@tbeu
Fix integer multiplication overflows
df23944 
As reported by #103 and https://github.com/TeamSeri0us/pocs/tree/master/matio
tbeu added a commit that referenced this issue Mar 20, 2019
@tbeu
Fix memory leak
ce33bf0 
As reported by #103
@tbeu tbeu closed this as completed Mar 20, 2019
@svillemot
Copy link
Contributor

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
Copy link
Contributor

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

@svillemot
Copy link
Contributor

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
Copy link
Contributor

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

@svillemot
Copy link
Contributor

svillemot commented Mar 22, 2019 •
edited
Loading

Applying adfa218 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 tbeu reopened this Mar 22, 2019
@tbeu
Copy link
Owner

tbeu commented Mar 22, 2019

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

@svillemot
Copy link
Contributor

It is good now, thanks!

@tbeu tbeu closed this as completed Mar 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@svillemot @carnil @tbeu @PhilipMorrisJones @cool-tomato @TeoShaw