kyz/libmspack

Malloc size error in chmd.c:1327:34

seviezhou opened this issue · 2 comments

System info

Ubuntu x86_64, clang 6.0, chmextract (latest master 7e63519)

Configure

CFLAGS="-g -fsanitize=address" LDFLAGS="-fsanitize=address" ./configure

Command line

./libmspack/examples/chmextract @@

AddressSanitizer output

==19777==WARNING: AddressSanitizer failed to allocate 0xffffffffffffc044 bytes
==19777==AddressSanitizer's allocator is terminating the process instead of returning 0
==19777==If you don't like this behavior set allocator_may_return_null=1
==19777==AddressSanitizer CHECK failed: /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:225 "((0)) != (0)" (0x0, 0x0)
    #0 0x4e7a9f in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/asan/asan_rtl.cc:69
    #1 0x504a15 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/sanitizer_common/sanitizer_termination.cc:79
    #2 0x4ed8c6 in __sanitizer::ReportAllocatorCannotReturnNull() /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:225
    #3 0x4ed903 in __sanitizer::ReturnNullOrDieOnFailure::OnBadRequest() /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:241
    #4 0x41e9d6 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/asan/asan_allocator.cc:856
    #5 0x4de584 in __interceptor_malloc /home/seviezhou/llvm-6.0.0/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:89
    #6 0x526156 in read_sys_file /home/seviezhou/libmspack/libmspack/mspack/chmd.c:1327:34
    #7 0x51ab64 in chmd_init_decomp /home/seviezhou/libmspack/libmspack/mspack/chmd.c:1059:16
    #8 0x51ab64 in chmd_extract /home/seviezhou/libmspack/libmspack/mspack/chmd.c:974
    #9 0x517045 in main /home/seviezhou/libmspack/libmspack/examples/chmextract.c:104:17
    #10 0x7f52106ae83f in __libc_start_main /build/glibc-e6zv40/glibc-2.23/csu/../csu/libc-start.c:291
    #11 0x41a398 in _start (/home/seviezhou/libmspack/libmspack/examples/chmextract+0x41a398)

POC

malloc-size-error-read_sys_file-chmd-1327.zip

kyz commented

Thank you for the report.

What happens here is that the file size (u64) is cast to an int (i32), and then cast to size_t (u32 or u64), so if the ControlData or ResetTable file size claims to be over 2GB, the int will be negative and the memory requested will either be 2-4 gigabytes (32-bit size_t) or 15-16 exabytes (64-bit size_t). It's unlikely this memory request will ever be fulfilled, but if it succeeded, the subsequent read() call would fail and the memory would be deallocated and the file rejected.

However, this report reveals a more concerning issue: when the ControlData or ResetTable file length is less than 2GB, but still a large value, and there is genuinely that much data provided for the file (so read() succeeds), it allows the file creator to make libmspack allocate and use up to 4GB of memory. This is against library user expectations, so the upper limit on memory use for these system files should be quantified.

read_sys_file() is used to read 3 system files into memory: SpanInfo, ControlData and ResetTable. SpanInfo is already rejected if its file length is not exactly 8 bytes.

ControlData is not loaded if its file length is less than 28 bytes, but larger files were allowed in case a new version of the file was introduced. As no new version has been introduced in years, it is much better to reject the file if not exactly 28 bytes. This does not affect any genuine CHM files.

ResetTable can theoretically be huge; the longest possible LZX stream (16 exabytes) could have a 4 petabyte ResetTable. But practically, the largest seen in the wild is 46 kilobytes (PHP manuals). so I picked an arbitrary upper limit of 1MB, which allows for decompressed LZX streams of over 4GB.

This is fixed in commit 2c4bf97, please confirm.

Thanks for your very detailed analysis!

I have tried the newest commit, this bug has been fixed. Good work!