LG MRA58K - Missing Bounds-Checking in AVI Stream Parsing

Source: https://bugs.chromium.org/p/project-zero/issues/detail?id=1206

Missing bounds-checking in AVI stream parsing

When parsing AVI files, CAVIFileParser uses the stream count from the AVI header 
to allocate backing storage for storing metadata about the streams (member 
variable m_aStream). However, the number of stream headers we parse is never 
validated against this allocation size during parsing, so we can write further 
metadata past the end of this buffer by constructing a file which contains more
stream headers than expected.

The allocation happens here:

int CAVIFileParser::ParseChunkAviHdr(int a2, unsigned int chunk_size)
{
  struct AviHeader *avih;
  int result;

  // snip some sanity checking (have we already found an 'avih' chunk, is this
  // chunk large enough to contain an avi header.)

  result = AVISourceReader::AVI_fread(this->source, avih, sizeof(struct AviHeader), 1);
  if ( result <= 0 )
  {
    // snip...
  }
  else
  {
    stream_count = avih->dwStreams; // <-- this is an attacker-controlled count
    this->m_aStreamCount = stream_count;
    this->m_aStream = malloc(stream_count * sizeof(struct AviStream));
    this->m_aStreamIndex = -1;

    // snip...
  }

  return 1;
}

There doesn't appear to be any integer overflow checking in the multiplication
either; so if the current issue is directly fixed there could still be a 
vulnerability if stream_count * sizeof(struct AviStream) overflows.

this->m_aStreamIndex is incremented without checking in 
CAVIFileParser::ParseChild and used as an index into m_aStream in several places
without checking, including in CAVIFileParser::ParseChunkStrHdr and
CAVIFileParser::ParseChunkStrFmt.

Several of the values that we can get written out of bounds are pointers to 
controlled data, which is an interesting exploitation primitive. I've attached
a PoC file and script to generate it which results in overlapping a SRIFFNode* 
with the contents of a 'strf' chunk, resulting in a free of an attacker 
controlled pointer - in this case, 0x41414141. Since the structure sizes are 
dependent on the version of the library, this may not work on different builds, 
but it will hopefully cause a crash regardless.

Build fingerprint: 'lge/p1_global_com/p1:6.0/MRA58K/1624210305d45:user/release-keys'
Revision: '11'
ABI: 'arm'
pid: 19481, tid: 19585, name: Binder_2  >>> /system/bin/mediaserver <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4140007c
    r0 00000002  r1 00000012  r2 ffffffd0  r3 f6b572f0
AM write failed: Broken pipe
    r4 f6b572e8  r5 41414141  r6 f5fb6000  r7 41400000
    r8 f155c748  r9 f6b4a594  sl 00000001  fp f000081c
    ip 41400048  sp f00005f8  lr f6b2c7a7  pc f6b29826  cpsr 200f0030

backtrace:
    #00 pc 00055826  /system/lib/libc.so (ifree+49)
    #01 pc 000587a3  /system/lib/libc.so (je_free+374)
    #02 pc 000058f3  /system/lib/liblg_parser_avi.so (_ZN14CAVIFileParser15DeleteSRIFFNodeEP9SRIFFNode+54)
    #03 pc 00005915  /system/lib/liblg_parser_avi.so (_ZN14CAVIFileParser7DestroyEv+12)
    #04 pc 00005a33  /system/lib/liblg_parser_avi.so (_ZN14CAVIFileParserD1Ev+14)
    #05 pc 00005a45  /system/lib/liblg_parser_avi.so (_ZN14CAVIFileParserD0Ev+4)
    #06 pc 0000442f  /system/lib/liblg_parser_avi.so (_ZN9AVIParser5CloseEv+12)
    #07 pc 00025baf  /system/lib/libLGParserOSAL.so (_ZN7android14LGAVIExtractorD1Ev+26)


Proof of Concept:
https://gitlab.com/exploit-database/exploitdb-bin-sploits/-/raw/main/bin-sploits/42170.zip