search

justdan96 / tsMuxer

tsMuxer is a transport stream muxer for remuxing/muxing elementary streams, EVO/VOB/MPG, MKV/MKA, MP4/MOV, TS, M2TS to TS to M2TS. Supported video codecs H.264/AVC, H.265/HEVC, VC-1, MPEG2. Supported audio codecs AAC, AC3 / E-AC3(DD+), DTS/ DTS-HD.
Apache License 2.0
829 stars 140 forks source link

heap buffer under-read is found in IOContextDemuxer::skip_bytes #853

Closed iwashiira closed 3 months ago

iwashiira commented 3 months ago

Our fuzzer found heap buffer under-read in IOContextDemuxer in the current master(75c9cb3). PoC is here.

#include "bufferedReaderManager.h"
#include "vod_common.h"
#include "abstractDemuxer.h"
#include "movDemuxer.h"
#include <cstdint>
#include <fs/systemlog.h>

using namespace std;

BufferedReaderManager readManager(2, DEFAULT_FILE_BLOCK_SIZE, DEFAULT_FILE_BLOCK_SIZE + MAX_AV_PACKET_SIZE,
                                  DEFAULT_FILE_BLOCK_SIZE / 2);

int main(int argc, char* argv[]) {

    string fileName = argv[1];
    AbstractDemuxer* demuxer = new MovDemuxer(readManager);

    uint32_t fileBlockSize = demuxer->getFileBlockSize();
    demuxer->openFile(fileName);
    int64_t discardedSize = 0;
    DemuxedData demuxedData;

    map<int32_t, TrackInfo> acceptedPidMap;
    demuxer->getTrackList(acceptedPidMap);

    PIDSet acceptedPidSet;
    for (const auto& itr : acceptedPidMap) acceptedPidSet.insert(itr.first);

    for (unsigned i = 0; i < DETECT_STREAM_BUFFER_SIZE / fileBlockSize; i++)
        demuxer->simpleDemuxBlock(demuxedData, acceptedPidSet, discardedSize);

    return 0;
}

Following is an output of ASAN. vuln11.mov is in poc11.zip

$ tsmuxer ../crash/vuln11.mov
=================================================================
==74412==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f69111ec78c at pc 0x7f69158c0397 bp 0x7ffc6cdc5fe0 sp 0x7ffc6cdc5788
READ of size 12066 at 0x7f69111ec78c thread T0
    #0 0x7f69158c0396 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x561e964bceff in IOContextDemuxer::get_buffer(unsigned char*, unsigned int) (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x336eff)
    #2 0x561e9659c805 in MovDemuxer::simpleDemuxBlock(std::map<int, MemoryBlock, std::less<int>, std::allocator<std::pair<int const, MemoryBlock> > >&, std::set<int, std::less<int>, std::allocator<int> > const&, long&) (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x416805)
    #3 0x561e964ea187 in main (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x364187)
    #4 0x7f6915232d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #5 0x7f6915232e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #6 0x561e963ff0d4 in _start (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x2790d4)

0x7f69111ec78c is located 116 bytes to the left of 2129920-byte region [0x7f69111ec800,0x7f69113f4800)
allocated by thread T0 here:
    #0 0x7f691593c357 in operator new[](unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x561e9644f183 in ReaderData::init() (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x2c9183)
    #2 0x561e9644f21f in ReaderData::openStream() (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x2c921f)
    #3 0x561e9644e530 in FileReaderData::openStream() (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x2c8530)
    #4 0x561e9644e75c in BufferedFileReader::openStream(int, char const*, int, CodecInfo const*) (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x2c875c)
    #5 0x561e965999e3 in MovDemuxer::openFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x4139e3)
    #6 0x561e964e9ea3 in main (/home/vagrant/tsmuxer/for_build/build/tsMuxer/tsmuxer+0x363ea3)
    #7 0x7f6915232d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x0feda22358a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feda22358b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feda22358c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feda22358d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feda22358e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0feda22358f0: fa[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0feda2235900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feda2235910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feda2235920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feda2235930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0feda2235940: 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
==74412==ABORTING

It is caused by this function. https://github.com/justdan96/tsMuxer/blob/5f43ab2a45482ad448524dc61a1ab7204ca8849d/tsMuxer/ioContextDemuxer.cpp#L121-L158 In IOContextDemuxer::skip_bytes(const int64_t size), unlike get_buffer, the type around copyLen is performed in int64_t. This allows passing any negative number as the value returned by the result of the min macro, and there is a buffer under-read that can memcpy data from the memory area by subtracting m_curPos as desired based on this value.

Ricerca Security, Inc.

iwashiira commented 3 months ago

Looking at the commit history, skipLeft and size originally used uint64_t, but this commit changes them from uint64_t to int64_t. https://github.com/justdan96/tsMuxer/commit/37f4eb5fa0f0861704786e6367987a1923477735

Depending on the intent of this change, it may be better to consider a different mitigation.