[FFmpeg-devel] [PATCH] matroskadec: fix NULL pointer dereference

[Andreas Cadhalpun]

On 17.10.2016 15:44, James Almer wrote:
> On 10/17/2016 4:47 AM, Benoit Fouet wrote:
>> On 17/10/2016 06:49, James Almer wrote:
>>> On 10/16/2016 9:30 PM, James Almer wrote:
>>>> On 10/16/2016 5:11 PM, Andreas Cadhalpun wrote:
>>>>> The problem was introduced in commit 1273bc6.
>>>>>
>>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>>> ---
>>>>>   libavformat/matroskadec.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>>>> index 8847c62..a5d3c0e 100644
>>>>> --- a/libavformat/matroskadec.c
>>>>> +++ b/libavformat/matroskadec.c
>>>>> @@ -1759,7 +1759,7 @@ static int mkv_field_order(MatroskaDemuxContext *matroska, int64_t field_order)
>>>>>         /* workaround a bug in our Matroska muxer, introduced in version 57.36 alongside
>>>>>        * this function, and fixed in 57.52 */
>>>>> -    if (sscanf(matroska->muxingapp, "Lavf%d.%d.%d", &major, &minor, &micro) == 3)
>>>>> +    if (matroska->muxingapp && sscanf(matroska->muxingapp, "Lavf%d.%d.%d", &major, &minor, &micro) == 3)
>>>> LGTM.
>>>>
>>>> Matroska files are supposed to always have that element, but even ffmpeg used
>>>> to mux files without it at some point when bitexact flag was enabled, so i
>>>> guess plenty of files out there are missing it.
>>>>
>>>>>           bttb = (major == 57 && minor >= 36 && minor <= 51 && micro >= 100);
>>>>>         switch (field_order) {
>>>>>
>>> Just tried a file missing the muxingapp element, meaning matroska->muxingapp
>>> is NULL, and sscanf simply returns -1 and sets errno to EINVAL.
>>>
>>> Where does it crash for you and using what file?
>>
>> FWIW, I've just written a quick test on my machine, and an "sscanf(NULL, "%d", &i)" segfaults.

With glibc it crashes:
$ gdb --batch -ex r -ex bt -ex q --args ./ffmpeg_g -i ./id_b42167853bffe74a3a93ea312837dd8a2a25a951.matroska -f null /dev/null
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
ffmpeg version N-82012-gd3874b7 Copyright (c) 2000-2016 the FFmpeg developers
  built with gcc 6.2.0 (Debian 6.2.0-6) 20161010
  configuration: --disable-optimizations
  libavutil      55. 32.100 / 55. 32.100
  libavcodec     57. 61.103 / 57. 61.103
  libavformat    57. 52.100 / 57. 52.100
  libavdevice    57.  0.102 / 57.  0.102
  libavfilter     6. 64.100 /  6. 64.100
  libswscale      4.  1.100 /  4.  1.100
  libswresample   2.  2.100 /  2.  2.100
[matroska,webm @ 0x2380fc0] Unknown EBML doctype 'ma at roUka'
Truncating packet of size 134217728 to 1551
Truncating packet of size 1431346 to 1507
Truncating packet of size 4467 to 1401
[matroska,webm @ 0x2380fc0] Duplicate element
[matroska,webm @ 0x2380fc0] Unknown/unsupported AVCodecID ��OPU=.
Ignoring attempt to set invalid timebase 0/1 for st:0

Program received signal SIGSEGV, Segmentation fault.
rawmemchr () at ../sysdeps/x86_64/rawmemchr.S:37
37	../sysdeps/x86_64/rawmemchr.S: Datei oder Verzeichnis nicht gefunden.
#0  rawmemchr () at ../sysdeps/x86_64/rawmemchr.S:37
#1  0x00007ffff4b383a2 in _IO_str_init_static_internal (sf=sf at entry=0x7fffffffd360, ptr=ptr at entry=0x0, size=size at entry=0, pstart=pstart at entry=0x0) at strops.c:41
#2  0x00007ffff4b27567 in __GI___isoc99_vsscanf (string=0x0, format=0x15e597c "Lavf%d.%d.%d", args=args at entry=0x7fffffffd488) at isoc99_vsscanf.c:41
#3  0x00007ffff4b27507 in __isoc99_sscanf (s=<optimized out>, format=<optimized out>) at isoc99_sscanf.c:31
#4  0x00000000006ca2eb in mkv_field_order (matroska=0x2381ba0, field_order=2) at src/libavformat/matroskadec.c:1762
#5  0x00000000006cc0c2 in matroska_parse_tracks (s=0x2380fc0) at src/libavformat/matroskadec.c:2293
#6  0x00000000006ccc83 in matroska_read_header (s=0x2380fc0) at src/libavformat/matroskadec.c:2470
#7  0x00000000007bbd13 in avformat_open_input (ps=0x7fffffffd948, filename=0x7fffffffe3a4 "./id_b42167853bffe74a3a93ea312837dd8a2a25a951.matroska", fmt=0x0, options=0x2380d68) at src/libavformat/utils.c:587
#8  0x0000000000411aeb in open_input_file (o=0x7fffffffda50, filename=0x7fffffffe3a4 "./id_b42167853bffe74a3a93ea312837dd8a2a25a951.matroska") at src/ffmpeg_opt.c:997
#9  0x000000000041a9af in open_files (l=0x2380c98, inout=0x154c797 "input", open_file=0x411330 <open_input_file>) at src/ffmpeg_opt.c:3135
#10 0x000000000041ab10 in ffmpeg_parse_options (argc=6, argv=0x7fffffffe048) at src/ffmpeg_opt.c:3175
#11 0x000000000042e78a in main (argc=6, argv=0x7fffffffe048) at src/ffmpeg.c:4564

> Guess it's up to the implementation, then. I tried with mingw-w64 only.
> The documentation for sscanf says it sets errno to EINVAL if *format is NULL,
> but doesn't mention *str.
> 
> Patch still LGTM. It doesn't hurt either way.

Pushed.

Best regards,
Andreas