Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

global-buffer-overflow in function parse() in frontend/mp4read.c:746 #34

Closed
hlef opened this issue May 5, 2019 · 0 comments
Closed

Comments

@hlef
Copy link
Contributor

hlef commented May 5, 2019

Dear FAAD2 developers,

Looks like this issue didn't have a bug report yet. Originally reported on sourceforge, still affecting the master.

Link to poc.

I have a patch pending, will PR soon. This will also address #13.

fish@ubuntu: ./afl/afl/bin/faad global-buffer-overflow-1
 *********** Ahead Software MPEG-4 AAC Decoder V2.8.8 ******************

 Build: Nov 11 2018
 Copyright 2002-2004: Ahead Software AG
 http://www.audiocoding.com
 bug tracking: https://sourceforge.net/p/faac/bugs/
 Floating point version

 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License.

 **************************************************************************

**** MP4 header ****
Brand:			mp42(version 0)
Compatible brands:	mp42isom
*track media type: 'soun': OK
=================================================================
==73817==ERROR: AddressSanitizer: global-buffer-overflow on address 0x56118d728230 at pc 0x56118d4fdb1d bp 0x7ffd93a74c40 sp 0x7ffd93a74c30
READ of size 2 at 0x56118d728230 thread T0
    #0 0x56118d4fdb1c in parse ../../frontend/mp4read.c:746
    #1 0x56118d505ce2 in mp4read_open ../../frontend/mp4read.c:991
    #2 0x56118d517624 in decodeMP4file ../../frontend/main.c:830
    #3 0x56118d517624 in faad_main ../../frontend/main.c:1308
    #4 0x7f4a23581b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #5 0x56118d4fce69 in _start (/home/fish/Desktop/2018-10-10/sound_audio/faad2/afl/afl/bin/faad+0xae69)

0x56118d728230 is located 48 bytes to the left of global variable 'mvhd' defined in '../../frontend/mp4read.c:802:22' (0x56118d728260) of size 32
0x56118d728230 is located 0 bytes to the right of global variable 'trak' defined in '../../frontend/mp4read.c:806:22' (0x56118d728020) of size 528
SUMMARY: AddressSanitizer: global-buffer-overflow ../../frontend/mp4read.c:746 in parse
Shadow bytes around the buggy address:
  0x0ac2b1adcff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac2b1add000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac2b1add010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac2b1add020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac2b1add030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ac2b1add040: 00 00 00 00 00 00[f9]f9 f9 f9 f9 f9 00 00 00 00
  0x0ac2b1add050: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac2b1add060: 00 00 00 00 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x0ac2b1add070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac2b1add080: f9 f9 f9 f9 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9
  0x0ac2b1add090: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 00 00 00 00
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
==73817==ABORTING
hlef added a commit to hlef/faad2 that referenced this issue May 5, 2019
moovin contains calls to parse() and modifies the value of g_atom,
which is a global, static variable. When moovin returns, the position
of the g_atom pointer has thus changed. For example it might be
pointing to the last element of a trak[], meaning that g_atom++
(mp4read.c:765) pushes it out-of-bounds.

the result is a buffer-over-read later at line 767.

fix: moovin should not modify its caller's g_atom. Save g_atom's value
at the beginning of moovin calls and reset it before returning.

fixes knik0#34.
hlef added a commit to hlef/faad2 that referenced this issue Aug 10, 2019
moov blocks contain MP4 file metadata. mp4read.c parses moov blocks
using moovin, which recursively calls parse to process subblocks.  In
order to use the parse function, moovin modifies the value of g_atom,
which is a global, static variable. When moovin returns, the position
of the g_atom pointer has thus changed. For example it might be
pointing to the last element of a trak[], meaning that g_atom++
pushes it out-of-bounds.

the result is a buffer-over-read later at line 767.

fix: moovin should not modify g_atom from the point of view of its
caller. Save g_atom's value at the beginning of moovin calls and reset
it before returning.

fixes knik0#34.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant