heap buffer overflow is found in MatroskaDemuxer::matroska_parse_block()

[JP3BGY]

Our fuzzer found heap bof in MatroskaDemuxer::matroska_parse_block() in the current main(5f43ab2). PoC is here.

#include "bufferedReaderManager.h"
#include "vod_common.h"
#include "abstractDemuxer.h"
#include "matroskaDemuxer.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 MatroskaDemuxer(readManager);

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

  PIDSet acceptedPidSet;

  demuxer->simpleDemuxBlock(demuxedData, acceptedPidSet, discardedSize);

  return 0;
}

Following is an output of ASAN. poc.mkv is in lace_size_poc.zip.

[!] [ForkServer] Failed to get executor id: Bad file descriptor
        Tips: Is this forkserver attached to client?
        Just executing program...
=================================================================
==81==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000007d4 at pc 0x55dd625c7934 bp 0x7ffc8242ee30 sp 0x7ffc8242ee28
WRITE of size 4 at 0x6020000007d4 thread T0
    #0 0x55dd625c7933 in MatroskaDemuxer::matroska_parse_block(unsigned char*, int, long, long, long, int, int) /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:600:26
    #1 0x55dd625a6940 in MatroskaDemuxer::matroska_parse_cluster() /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:954:23
    #2 0x55dd625a3f2b in MatroskaDemuxer::readPacket(AVPacket&) /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:1125:28
    #3 0x55dd6259735c in MatroskaDemuxer::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&) /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:2377:29
    #4 0x55dd62594f16 in main /src/tsMuxer/tsMuxer/main.cpp:23:12
    #5 0x7f87a4a56d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #6 0x7f87a4a56e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #7 0x55dd622ce484 in _start (/out/tsmuxer-mkv2+0x119484) (BuildId: 8a3bffd4a84d0e3e)

0x6020000007d4 is located 0 bytes to the right of 4-byte region [0x6020000007d0,0x6020000007d4)
allocated by thread T0 here:
    #0 0x55dd6238e21d in operator new[](unsigned long) (/out/tsmuxer-mkv2+0x1d921d) (BuildId: 8a3bffd4a84d0e3e)
    #1 0x55dd625c5b29 in MatroskaDemuxer::matroska_parse_block(unsigned char*, int, long, long, long, int, int) /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:542:49
    #2 0x55dd625a6940 in MatroskaDemuxer::matroska_parse_cluster() /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:954:23
    #3 0x55dd625a3f2b in MatroskaDemuxer::readPacket(AVPacket&) /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:1125:28
    #4 0x55dd6259735c in MatroskaDemuxer::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&) /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:2377:29
    #5 0x55dd62594f16 in main /src/tsMuxer/tsMuxer/main.cpp:23:12
    #6 0x7f87a4a56d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/tsMuxer/tsMuxer/matroskaDemuxer.cpp:600:26 in MatroskaDemuxer::matroska_parse_block(unsigned char*, int, long, long, long, int, int)
Shadow bytes around the buggy address:
  0x0c047fff80a0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 00 07
  0x0c047fff80b0: fa fa fd fd fa fa fd fa fa fa 00 04 fa fa fd fa
  0x0c047fff80c0: fa fa fd fd fa fa fd fa fa fa 00 04 fa fa fd fa
  0x0c047fff80d0: fa fa fd fa fa fa fd fa fa fa 00 07 fa fa fd fa
  0x0c047fff80e0: fa fa fd fa fa fa 00 05 fa fa fd fa fa fa fd fa
=>0x0c047fff80f0: fa fa fd fa fa fa 00 05 fa fa[04]fa fa fa fa fa
  0x0c047fff8100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8140: 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
==81==ABORTING

It is caused by these lines because n and laces is from user data. https://github.com/justdan96/tsMuxer/blob/75c9cb3514815d07378007d36cc90c3f209e7b36/tsMuxer/matroskaDemuxer.cpp#L542-L600

Ricerca Security, Inc.

[jcdr428]

Ok, I reopen the issue as the bof needs another solution (see issue #874). EMBL lacing is explained here: lace and n are not really linked. The data buffer is composed of:

• Byte 1 = lace = Number of frames minus 1.
• Then we have the size of first frame in VINT format.
• Then we have the size of following frames except last one, as a difference from previous frame size.
• Then we have the data for each frame. Size of last frame is the remaining bytes.

So we have to check when ready each frame size, that this frame size is not > remaining buffer bytes.

[jcdr428]

Edit: @JP3BGY actually your POC is a specific case where there is only one frame in the EBML lacing. In which case, no need to deduct the size of the last frame from the remaining bytes. So we just need to add if (laces > 1) before lace_size[n] = size - total; to solve the bof...