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 |