[FFmpeg-devel] [FFmpeg-cvslog] avformat/mxfdec: prefer footer and complete partitions for metadata

[Marton Balint]

On Tue, 10 Aug 2021, Michael Niedermayer wrote:

> On Sun, Aug 01, 2021 at 01:15:32AM +0000, Marton Balint wrote:
>> ffmpeg | branch: master | Marton Balint <cus at passwd.hu> | Sun Jun 27 22:59:49 2021 +0200| [7b4bdcd68e1e0abfab21a8be81789531d649c1ff] | committer: Marton Balint
>>
>> avformat/mxfdec: prefer footer and complete partitions for metadata
>>
>> Also do not store inferior metadata with the same UID.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=7b4bdcd68e1e0abfab21a8be81789531d649c1ff
>> ---
>>
>>  libavformat/mxfdec.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 47 insertions(+), 4 deletions(-)
> [...]
>> @@ -842,10 +855,39 @@ static int mxf_read_partition_pack(void *arg, AVIOContext *pb, int tag, int size
>>      return 0;
>>  }
>>
>> +static int partition_score(MXFPartition *p)
>> +{
>> +    if (p->type == Footer)
>
> This can fail both as null pointer dereference from mxf->current_partition
> being NULL as well as a read after free from a realloc

Hmm, thanks, strange how I (and fate) missed this so obvious problem. Will 
send a fix.

Regards,
Marton

>
>
> here are the 2 traces:
> ==15334==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x000000cb211e bp 0x7ffde58f6780 sp 0x7ffde58f6760 T0)
> ==15334==The signal is caused by a READ memory access.
> ==15334==Hint: address points to the zero page.
>    #0 0xcb211d in partition_score ffmpeg/libavformat/mxfdec.c:860:12
>    #1 0xcb149e in mxf_add_metadata_set ffmpeg/libavformat/mxfdec.c:882:29
>    #2 0xc7e98c in mxf_read_local_tags ffmpeg/libavformat/mxfdec.c:3004:19
>    #3 0xc7e98c in mxf_parse_klv ffmpeg/libavformat/mxfdec.c:3031
>    #4 0xc69296 in mxf_read_header ffmpeg/libavformat/mxfdec.c:3445:28
>    #5 0xff3e67 in avformat_open_input ffmpeg/libavformat/utils.c:571:20
>    #6 0x4c779c in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:187:11
>    #7 0x271b34d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13
>    #8 0x270ff22 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6
>    #9 0x2715121 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9
>    #10 0x270fc00 in main Fuzzer/build/../FuzzerMain.cpp:20:10
>    #11 0x7ff2d603ebf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
>    #12 0x41fb79 in _start (ffmpeg/tools/target_io_dem_fuzzer+0x41fb79)
>
> =================================================================
> ==15313==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000006d8 at pc 0x000000d6eca2 bp 0x7ffd92ec0950 sp 0x7ffd92ec0948
> READ of size 4 at 0x6120000006d8 thread T0
>    #0 0xd6eca1 in partition_score ffmpeg/libavformat/mxfdec.c:860:12
>    #1 0xd6deee in mxf_add_metadata_set ffmpeg/libavformat/mxfdec.c:882:29
>    #2 0xd3b3dc in mxf_read_local_tags ffmpeg/libavformat/mxfdec.c:3004:19
>    #3 0xd3b3dc in mxf_parse_klv ffmpeg/libavformat/mxfdec.c:3031
>    #4 0xd25ce6 in mxf_read_header ffmpeg/libavformat/mxfdec.c:3445:28
>    #5 0x4f2707 in avformat_open_input ffmpeg/libavformat/utils.c:571:20
>    #6 0x4c6c35 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:187:11
>    #7 0x271a86d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13
>    #8 0x270f442 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6
>    #9 0x2714641 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9
>    #10 0x270f120 in main Fuzzer/build/../FuzzerMain.cpp:20:10
>    #11 0x7fbf99d16bf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
>    #12 0x41fb79 in _start (ffmpeg/tools/target_dem_fuzzer+0x41fb79)
>
> 0x6120000006d8 is located 152 bytes inside of 288-byte region [0x612000000640,0x612000000760)
> freed by thread T0 here:
>    #0 0x497e19 in realloc /b/swarming/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:164:3
>    #1 0xd5e0f4 in mxf_read_partition_pack ffmpeg/libavformat/mxfdec.c:700:16
>    #2 0xd3a842 in mxf_parse_klv ffmpeg/libavformat/mxfdec.c:3034:15
>    #3 0xd25ce6 in mxf_read_header ffmpeg/libavformat/mxfdec.c:3445:28
>    #4 0x4f2707 in avformat_open_input ffmpeg/libavformat/utils.c:571:20
>    #5 0x4c6c35 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:187:11
>    #6 0x271a86d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13
>    #7 0x270f442 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6
>    #8 0x2714641 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9
>    #9 0x270f120 in main Fuzzer/build/../FuzzerMain.cpp:20:10
>    #10 0x7fbf99d16bf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
>
> previously allocated by thread T0 here:
>    #0 0x497e19 in realloc /b/swarming/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:164:3
>    #1 0xd5e0f4 in mxf_read_partition_pack ffmpeg/libavformat/mxfdec.c:700:16
>    #2 0xd3a842 in mxf_parse_klv ffmpeg/libavformat/mxfdec.c:3034:15
>    #3 0xd25ce6 in mxf_read_header ffmpeg/libavformat/mxfdec.c:3445:28
>    #4 0x4f2707 in avformat_open_input ffmpeg/libavformat/utils.c:571:20
>    #5 0x4c6c35 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:187:11
>    #6 0x271a86d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13
>    #7 0x270f442 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6
>    #8 0x2714641 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9
>    #9 0x270f120 in main Fuzzer/build/../FuzzerMain.cpp:20:10
>    #10 0x7fbf99d16bf6 in __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:310
>
>    [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The greatest way to live with honor in this world is to be what we pretend
> to be. -- Socrates
>