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.

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

Multiple heap-based buffer overflows in CmtkLoader::load() #90

Closed
fcambus opened this issue Aug 6, 2019 · 1 comment
Closed

Multiple heap-based buffer overflows in CmtkLoader::load() #90

fcambus opened this issue Aug 6, 2019 · 1 comment

Comments

@fcambus
Copy link
Contributor

fcambus commented Aug 6, 2019

Hi,

While fuzzing AdPlug with American Fuzzy Lop, I found multiple heap-based buffer overflows in CmtkLoader::load(), in src/mtk.cpp L95 and L103.

Attaching reproducers for both issues (gzipped so GitHub accepts them):

=================================================================
==7055==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x628000003845 at pc 0x0000004c6aaf bp 0x7fff98d6e270 sp 0x7fff98d6da20
WRITE of size 3897 at 0x628000003845 thread T0
    #0 0x4c6aae in __asan_memset (/home/fcambus/reps/adplay+0x4c6aae)
    #1 0x7f4ce0d85ad7 in CmtkLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&) /home/fcambus/adplug/src/mtk.cpp:95:7
    #2 0x7f4ce0cc61d5 in CAdPlug::factory(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Copl*, CPlayers const&, CFileProvider const&) /home/fcambus/adplug/src/adplug.cpp:169:10
    #3 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #4 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #5 0x7f4ce050409a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #6 0x41f759 in _start (/home/fcambus/reps/adplay+0x41f759)

0x628000003845 is located 0 bytes to the right of 14149-byte region [0x628000000100,0x628000003845)
allocated by thread T0 here:
    #0 0x4f6972 in operator new[](unsigned long) (/home/fcambus/reps/adplay+0x4f6972)
    #1 0x7f4ce0d85223 in CmtkLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&) /home/fcambus/adplug/src/mtk.cpp:60:9
    #2 0x7f4ce0cc61d5 in CAdPlug::factory(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Copl*, CPlayers const&, CFileProvider const&) /home/fcambus/adplug/src/adplug.cpp:169:10
    #3 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #4 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #5 0x7f4ce050409a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/fcambus/reps/adplay+0x4c6aae) in __asan_memset
Shadow bytes around the buggy address:
  0x0c507fff86b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fff86c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fff86d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fff86e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c507fff86f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c507fff8700: 00 00 00 00 00 00 00 00[05]fa fa fa fa fa fa fa
  0x0c507fff8710: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c507fff8720: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c507fff8730: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c507fff8740: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c507fff8750: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Shadow gap:              cc
==7055==ABORTING
=================================================================
==7059==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x607000000065 at pc 0x0000004c6798 bp 0x7ffc61e0a6b0 sp 0x7ffc61e09e60
WRITE of size 84 at 0x607000000065 thread T0
    #0 0x4c6797 in __asan_memcpy (/home/fcambus/reps/adplay+0x4c6797)
    #1 0x7f37d10fdd0a in CmtkLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&) /home/fcambus/adplug/src/mtk.cpp:103:7
    #2 0x7f37d103e1d5 in CAdPlug::factory(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Copl*, CPlayers const&, CFileProvider const&) /home/fcambus/adplug/src/adplug.cpp:169:10
    #3 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #4 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #5 0x7f37d087c09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #6 0x41f759 in _start (/home/fcambus/reps/adplay+0x41f759)

0x607000000065 is located 0 bytes to the right of 69-byte region [0x607000000020,0x607000000065)
allocated by thread T0 here:
    #0 0x4f6972 in operator new[](unsigned long) (/home/fcambus/reps/adplay+0x4f6972)
    #1 0x7f37d10fd223 in CmtkLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&) /home/fcambus/adplug/src/mtk.cpp:60:9
    #2 0x7f37d103e1d5 in CAdPlug::factory(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Copl*, CPlayers const&, CFileProvider const&) /home/fcambus/adplug/src/adplug.cpp:169:10
    #3 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #4 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #5 0x7f37d087c09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/fcambus/reps/adplay+0x4c6797) in __asan_memcpy
Shadow bytes around the buggy address:
  0x0c0e7fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0e7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0e7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0e7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0e7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c0e7fff8000: fa fa fa fa 00 00 00 00 00 00 00 00[05]fa fa fa
  0x0c0e7fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8050: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Shadow gap:              cc
==7059==ABORTING
@fcambus
Copy link
Contributor Author

fcambus commented Aug 7, 2019

This issue has been assigned CVE-2019-14734.

miller-alex added a commit to miller-alex/adplug that referenced this issue Apr 3, 2020
While fuzzing AdPlug with American Fuzzy Lop, Frederic Cambus
found several memory issues and reported them on github. Hook
up the reproducers he provided as test cases in stresstest.cpp.
This includes tests for the following github issues:

* Issue adplug#85 ("Heap-based buffer overflow in
  CxadbmfPlayer::__bmf_convert_stream()")
* Issue adplug#86 ("Heap-based buffer overflow in CdtmLoader::load()")
* Issue adplug#87 ("Heap-based buffer overflow in CmkjPlayer::load()")
* Issue adplug#88 ("Multiple heap-based buffer overflows in Ca2mLoader::load()")
* Issue adplug#89 ("Multiple heap-based buffer overflows in CradLoader::load()")
* Issue adplug#90 ("Multiple heap-based buffer overflows in CmtkLoader::load()")
* Issue adplug#91 ("Double free in Cu6mPlayer::~Cu6mPlayer()")

Co-authored-by: Frederic Cambus <fred@statdns.com>
Bug: adplug#85
Bug: adplug#86
Bug: adplug#87
Bug: adplug#88
Bug: adplug#89
Bug: adplug#90
Bug: adplug#91
miller-alex added a commit to miller-alex/adplug that referenced this issue Apr 3, 2020
Changes in src/mtk.cpp for loading files:
* Fail early if the (decompressed) size is too small to hold
  mtkdata minus patterns. That avoids attempts to copy data
  from beyond allocated memory.
* In the data decompression section, there are multiple cases
  where the code actually has checks for available space before
  copying data, but the size of the copy is increased after
  the check, so a buffer overflow is still possible (issue adplug#90).
  Fix that by moving the check after the size computation,
  and also check for a valid source offset where applicable.
* Also add several checks whether source data is exhausted
  during decompession, so
* When copying the patterns, don't copy more data than the
  "pattern" array can hold.

In src/mtk.h, method getinstrument(), check for valid instrument
number to avoid accessing the array with an invalid index.

This commit fixes CVE-2019-14734.

Fixes: adplug#90
Malvineous pushed a commit that referenced this issue May 11, 2020
While fuzzing AdPlug with American Fuzzy Lop, Frederic Cambus
found several memory issues and reported them on github. Hook
up the reproducers he provided as test cases in stresstest.cpp.
This includes tests for the following github issues:

* Issue #85 ("Heap-based buffer overflow in
  CxadbmfPlayer::__bmf_convert_stream()")
* Issue #86 ("Heap-based buffer overflow in CdtmLoader::load()")
* Issue #87 ("Heap-based buffer overflow in CmkjPlayer::load()")
* Issue #88 ("Multiple heap-based buffer overflows in Ca2mLoader::load()")
* Issue #89 ("Multiple heap-based buffer overflows in CradLoader::load()")
* Issue #90 ("Multiple heap-based buffer overflows in CmtkLoader::load()")
* Issue #91 ("Double free in Cu6mPlayer::~Cu6mPlayer()")

Co-authored-by: Frederic Cambus <fred@statdns.com>
Bug: #85
Bug: #86
Bug: #87
Bug: #88
Bug: #89
Bug: #90
Bug: #91
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants