#1127 an off by one vulnerability when parsing MpegPs file format

[casperslei]

There is an off by one vulnerability when parsing MpegPs file format.

ASAN output

=================================================================
==114347==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffecf740d58 at pc 0x00000122cbd9 bp 0x7ffecf740cb0 sp 0x7ffecf740ca8
READ of size 8 at 0x7ffecf740d58 thread T0
    #0 0x122cbd8 in MediaInfoLib::File_MpegPs::Streams_Fill_PerStream(unsigned long, MediaInfoLib::File_MpegPs::ps_stream&, MediaInfoLib::File_MpegPs::kindofstream) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Multiple/File_MpegPs.cpp:408:9
    #1 0x1228c53 in MediaInfoLib::File_MpegPs::Streams_Fill() /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Multiple/File_MpegPs.cpp:268:9
    #2 0x1b10465 in MediaInfoLib::File__Analyze::Fill(char const*) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/File__Analyze.cpp:3040:5
    #3 0x1b33cea in MediaInfoLib::File__Analyze::Fill(MediaInfoLib::File__Analyze*) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/File__Analyze.cpp:3057:13
    #4 0x12fbfaf in MediaInfoLib::File_MpegTs::Read_Buffer_AfterParsing() /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Multiple/File_MpegTs.cpp:2023:21
    #5 0x1b03d19 in MediaInfoLib::File__Analyze::Open_Buffer_Continue_Loop() /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/File__Analyze.cpp:1162:5
    #6 0x1afe82e in MediaInfoLib::File__Analyze::Open_Buffer_Continue(unsigned char const*, unsigned long) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/File__Analyze.cpp:719:16
    #7 0x6b8b2b in MediaInfoLib::MediaInfo_Internal::Open_Buffer_Continue(unsigned char const*, unsigned long) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/MediaInfo_Internal.cpp:1375:11
    #8 0x17470b3 in MediaInfoLib::Reader_File::Format_Test_PerParser_Continue(MediaInfoLib::MediaInfo_Internal*) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Reader/Reader_File.cpp:759:24
    #9 0x1742313 in MediaInfoLib::Reader_File::Format_Test_PerParser(MediaInfoLib::MediaInfo_Internal*, std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Reader/Reader_File.cpp:313:12
    #10 0x173f1be in MediaInfoLib::Reader_File::Format_Test(MediaInfoLib::MediaInfo_Internal*, std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Reader/Reader_File.cpp:225:17
    #11 0x6833a7 in MediaInfoLib::MediaInfo_Internal::Entry() /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/MediaInfo_Internal.cpp:1121:29
    #12 0x656276 in MediaInfoLib::MediaInfo_Internal::Open(std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/MediaInfo_Internal.cpp:877:9
    #13 0x702e73 in MediaInfoLib::MediaInfoList_Internal::Entry() /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/MediaInfoList_Internal.cpp:215:17
    #14 0x6fef54 in MediaInfoLib::MediaInfoList_Internal::Open(std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&, MediaInfoLib::fileoptions_t) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/MediaInfoList_Internal.cpp:151:9
    #15 0x4fd14d in main /home/casper/mi/tmp/MediaInfo/Project/GNU/CLI/../../../Source/CLI/CLI_Main.cpp:154:25
    #16 0x7fc369c32b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #17 0x426469 in _start (/home/casper/mi/afl/mediainfodbg+0x426469)

Address 0x7ffecf740d58 is located in stack of thread T0 at offset 152 in frame
    #0 0x122b31f in MediaInfoLib::File_MpegPs::Streams_Fill_PerStream(unsigned long, MediaInfoLib::File_MpegPs::ps_stream&, MediaInfoLib::File_MpegPs::kindofstream) /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Multiple/File_MpegPs.cpp:334

  This frame has 5 object(s):
    [32, 64) 'ref.tmp.i'
    [96, 152) 'Counts' (line 335) <== Memory access at offset 152 overflows this variable
    [192, 224) 'LawRating' (line 355)
    [256, 288) 'Title' (line 358)
    [320, 352) 'ref.tmp42' (line 359)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /home/casper/mi/tmp/MediaInfoLib/Project/GNU/Library/../../../Source/MediaInfo/Multiple/File_MpegPs.cpp:408:9 in MediaInfoLib::File_MpegPs::Streams_Fill_PerStream(unsigned long, MediaInfoLib::File_MpegPs::ps_stream&, MediaInfoLib::File_MpegPs::kindofstream)
Shadow bytes around the buggy address:
  0x100059ee0150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100059ee0160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100059ee0170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100059ee0180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100059ee0190: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f8 f8 f8 f8
=>0x100059ee01a0: f2 f2 f2 f2 00 00 00 00 00 00 00[f2]f2 f2 f2 f2
  0x100059ee01b0: f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f8 f8 f2 f2 f2 f2
  0x100059ee01c0: f8 f8 f8 f8 f3 f3 f3 f3 00 00 00 00 00 00 00 00
  0x100059ee01d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100059ee01e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100059ee01f0: f1 f1 f1 f1 f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f8 f8
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
==114347==ABORTING

Analysis

array Counts is an array with Stream_Max length. But code trying to access Counts[Stream_Max], so off by one would occur.
code snippet in MediaInfoLib/Source/MediaInfo/Multiple/File_MpegPs.cpp
332 │ //--------------------------------------------------------------------------- 333 │ void File_MpegPs::Streams_Fill_PerStream(size_t StreamID, ps_stream &Temp, kindofstream KindOfStream) 334 │ { 335 │ size_t Counts[Stream_Max]; 336 │ for (size_t StreamKind=Stream_General+1; StreamKind<Stream_Max; StreamKind++) 337 │ Counts[StreamKind]=Count_Get((stream_t)StreamKind); 338 │ 339 │ //By the parser 340 │ StreamKind_Last=Stream_Max; 341 │ size_t Count=0; 342 │ if (!Temp.Parsers.empty() &

reproduce steps:

1. compile mediainfo with ASAN
2. run poc with command line mediainfo poc.m2ts

[Luigi Baldoni]

Is this still present in 20.08 ?

[Jerome Martinez]

We didn't work on that yet, but should be in next release.

[Jerome Martinez]

Fixed, in next version.