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 CradLoader::load() #89

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

Multiple heap-based buffer overflows in CradLoader::load() #89

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 CradLoader::load(), in src/rad.cpp L74 and L85.

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

=================================================================
==6854==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60c0000000c0 at pc 0x7f2a30e461de bp 0x7ffc8f048df0 sp 0x7ffc8f048de8
WRITE of size 1 at 0x60c0000000c0 thread T0
    #0 0x7f2a30e461dd in CradLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&) /home/fcambus/adplug/src/rad.cpp:74:40
    #1 0x7f2a30d651d5 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
    #2 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #3 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #4 0x7f2a305a309a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #5 0x41f759 in _start (/home/fcambus/reps3/adplay+0x41f759)

0x60c0000000c0 is located 0 bytes to the right of 128-byte region [0x60c000000040,0x60c0000000c0)
allocated by thread T0 here:
    #0 0x4f6972 in operator new[](unsigned long) (/home/fcambus/reps3/adplay+0x4f6972)
    #1 0x7f2a30e31b72 in CmodPlayer::realloc_order(unsigned long) /home/fcambus/adplug/src/protrack.cpp:555:11
    #2 0x7f2a30e31a37 in CmodPlayer::CmodPlayer(Copl*) /home/fcambus/adplug/src/protrack.cpp:50:3
    #3 0x7f2a30e47d76 in CradLoader::CradLoader(Copl*) /home/fcambus/adplug/./src/rad.h:30:5
    #4 0x7f2a30e45021 in CradLoader::factory(Copl*) /home/fcambus/adplug/src/rad.cpp:30:14
    #5 0x7f2a30d6511d 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:168:10
    #6 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #7 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #8 0x7f2a305a309a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fcambus/adplug/src/rad.cpp:74:40 in CradLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&)
Shadow bytes around the buggy address:
  0x0c187fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c187fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c187fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c187fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c187fff8000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c187fff8010: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
  0x0c187fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff8060: 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
==6854==ABORTING
=================================================================
==6856==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x612000022b05 at pc 0x7fa8afc16641 bp 0x7ffd67c96c10 sp 0x7ffd67c96c08
WRITE of size 1 at 0x612000022b05 thread T0
    #0 0x7fa8afc16640 in CradLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&) /home/fcambus/adplug/src/rad.cpp:85:26
    #1 0x7fa8afb351d5 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
    #2 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #3 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #4 0x7fa8af37309a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #5 0x41f759 in _start (/home/fcambus/reps3/adplay+0x41f759)

0x612000022b05 is located 5 bytes to the right of 320-byte region [0x6120000229c0,0x612000022b00)
allocated by thread T0 here:
    #0 0x4f6972 in operator new[](unsigned long) (/home/fcambus/reps3/adplay+0x4f6972)
    #1 0x7fa8afc01d57 in CmodPlayer::realloc_patterns(unsigned long, unsigned long, unsigned long) /home/fcambus/adplug/src/protrack.cpp:570:41
    #2 0x7fa8afc01a6d in CmodPlayer::CmodPlayer(Copl*) /home/fcambus/adplug/src/protrack.cpp:51:3
    #3 0x7fa8afc17d76 in CradLoader::CradLoader(Copl*) /home/fcambus/adplug/./src/rad.h:30:5
    #4 0x7fa8afc15021 in CradLoader::factory(Copl*) /home/fcambus/adplug/src/rad.cpp:30:14
    #5 0x7fa8afb3511d 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:168:10
    #6 0x4fcd62 in play(char const*, Player*, int) /home/fcambus/adplay-unix/src/adplay.cc:309:11
    #7 0x4fbaf4 in main /home/fcambus/adplay-unix/src/adplay.cc:544:5
    #8 0x7fa8af37309a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/fcambus/adplug/src/rad.cpp:85:26 in CradLoader::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CFileProvider const&)
Shadow bytes around the buggy address:
  0x0c247fffc510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffc520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffc530: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fffc540: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffc550: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fffc560:[fa]fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fffc570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffc580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffc590: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fffc5a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffc5b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==6856==ABORTING
@fcambus
Copy link
Contributor Author

fcambus commented Aug 7, 2019

This issue has been assigned CVE-2019-14733.

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
This patches several memory issues while loading .rad files
in src/rad.cpp:
* Simplify the code reading the descrition and ensure not
  to write past the end of "desc".
* Add several checks for errors reading the file and fail
  loading in that case.
* Check instument number before using it as index to the
  "inst" array to avoid an out-of bounds write.
* Check order length before writing data to the array. Fixes
  a heap-based buffer overflow (issue adplug#89).
* Check channel and row numbers before using them as indices
  for writing track data. Fixes another buffer overflow
  (issue adplug#89).

This fixes CVE-2019-14733.

Fixes: adplug#89
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