LG MRA58K - Missing Bounds-Checking in AVI Stream Parsing

2017-06-13 22:05:02

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://github.com/offensive-security/exploit-database-bin-sploits/raw/master/sploits/42170.zip

Fixes

No fixes

In order to submit a new fix you need to be registered.