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

[Benoit Fouet]

Hi,

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.

-- 
Ben