Opened 8 years ago
Closed 8 years ago
#5809 closed defect (fixed)
av_parser_parse2 overflows input buffer (mpegaudio)
Reported by: | driegel | Owned by: | |
---|---|---|---|
Priority: | minor | Component: | documentation |
Version: | git-master | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | yes | |
Analyzed by developer: | no |
Description
Summary of the bug:
When parsing mpegaudio, av_parser_parse2 doesn't respect input buffer size and read beyond its limit. This overflow can lead to crashes.
FFmpeg version:
n3.1.2 (built from sources)
How to reproduce:
Compile with make (adjusted the Makefile if necessary), and run output binary with valgrind.
Valgrind output:
$ valgrind ./test ==9820== Memcheck, a memory error detector ==9820== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==9820== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info ==9820== Command: ./test ==9820== ==9820== Invalid read of size 8 ==9820== at 0x4C326C8: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9820== by 0x55CED85: memcpy (string3.h:53) ==9820== by 0x55CED85: ff_combine_frame (parser.c:298) ==9820== by 0x55657F8: mpegaudio_parse (mpegaudio_parser.c:111) ==9820== by 0x55CE9A0: av_parser_parse2 (parser.c:182) ==9820== by 0x400D0A: main (test.c:67) ==9820== Address 0x76a2258 is 0 bytes after a block of size 40 alloc'd ==9820== at 0x4C2FFC6: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9820== by 0x4C300D1: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9820== by 0x63016CF: av_malloc (mem.c:97) ==9820== by 0x400C4D: main (test.c:53) ==9820== ==9820== Invalid read of size 8 ==9820== at 0x4C326D6: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9820== by 0x55CED85: memcpy (string3.h:53) ==9820== by 0x55CED85: ff_combine_frame (parser.c:298) ==9820== by 0x55657F8: mpegaudio_parse (mpegaudio_parser.c:111) ==9820== by 0x55CE9A0: av_parser_parse2 (parser.c:182) ==9820== by 0x400D0A: main (test.c:67) ==9820== Address 0x76a2f80 is 8 bytes after a block of size 40 alloc'd ==9820== at 0x4C2FFC6: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9820== by 0x4C300D1: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==9820== by 0x63016CF: av_malloc (mem.c:97) ==9820== by 0x400C4D: main (test.c:53) ==9820== ==9820== ==9820== HEAP SUMMARY: ==9820== in use at exit: 40 bytes in 1 blocks ==9820== total heap usage: 734 allocs, 733 frees, 67,939 bytes allocated ==9820== ==9820== LEAK SUMMARY: ==9820== definitely lost: 0 bytes in 0 blocks ==9820== indirectly lost: 0 bytes in 0 blocks ==9820== possibly lost: 0 bytes in 0 blocks ==9820== still reachable: 40 bytes in 1 blocks ==9820== suppressed: 0 bytes in 0 blocks ==9820== Rerun with --leak-check=full to see details of leaked memory ==9820== ==9820== For counts of detected and suppressed errors, rerun with: -v ==9820== ERROR SUMMARY: 92 errors from 2 contexts (suppressed: 0 from 0)
Quick link to checkout the code and run test case:
https://github.com/d-k-c/ffmpeg-av_parser_parse2-bug
Attachments (4)
Change History (10)
by , 8 years ago
Attachment: | encoded_audio.c added |
---|
by , 8 years ago
follow-up: 2 comment:1 by , 8 years ago
Component: | avcodec → undetermined |
---|---|
Resolution: | → invalid |
Status: | new → closed |
I believe the documentation requires you to pad the input buffer.
For future tickets, please remember to test current FFmpeg git head before reporting issues here.
comment:2 by , 8 years ago
Replying to cehoyos:
I believe the documentation requires you to pad the input buffer.
The documentation doesn't state any requirement about the input buffer. Compilation was broken with git head and the file (mpegaudio_parser.c) hasn't changed since the release I used, so I supposed it was a good approximate.
comment:3 by , 8 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
The documentation of av_parser_parse2() doesn't mention padding, I don't know if there is an issue.
comment:4 by , 8 years ago
Not reproducible with ffmpeg
because av_grow_packet() does indeed pad each AVPacket with AV_INPUT_BUFFER_PADDING_SIZE.
by , 8 years ago
Attachment: | patchparserpadding.diff added |
---|
comment:5 by , 8 years ago
IIRC the parsers always were written on the assumtation that the input is padded. So to me this looks like a documentation issue
Of course if someone wants to change all parsers to not require padding that would e welcome if its clean and has no speed penalty
comment:6 by , 8 years ago
Component: | undetermined → documentation |
---|---|
Priority: | normal → minor |
Reproduced by developer: | set |
Resolution: | → fixed |
Status: | reopened → closed |
Version: | unspecified → git-master |
Documentation improved in 7a8e5ff1fd06c0363ed4bb26cda5262fcd925b74, thank you for the report!
mp2-encoded audio dump