[FFmpeg-devel] [PATCH 1/4] avcodec/svq3: dont crash on free_picture(NULL)

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Andreas Rheinhardt:
>> Michael Niedermayer:
>>> Fixes: NULL dereference
>>> Fixes: 25762/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SVQ3_fuzzer-5716279070294016
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavcodec/svq3.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
>>> index fb7b992496..2da757bee9 100644
>>> --- a/libavcodec/svq3.c
>>> +++ b/libavcodec/svq3.c
>>> @@ -1323,6 +1323,10 @@ static av_cold int svq3_decode_init(AVCodecContext *avctx)
>>>  static void free_picture(AVCodecContext *avctx, SVQ3Frame *pic)
>>>  {
>>>      int i;
>>> +
>>> +    if (!pic)
>>> +        return;
>>> +
>>>      for (i = 0; i < 2; i++) {
>>>          av_freep(&pic->motion_val_buf[i]);
>>>      }
>>>
>> This surprises me a lot: Since 96061c5a4f690c3ab49e4458701bb013fd3dd57f,
>> the pointers to the pictures are set to something different from NULL
>> directly at the beginning of the init function; they are never set to
>> NULL afterwards, they are only swapped with pointers to other pictures.
>> So how can they ever be NULL? Has the init function not been properly
>> called? (And if any of these pictures were NULL, then svq3_decode_end()
>> will call av_frame_free(NULL), which is unsafe, but doesn't crash
>> currently; the situation would change if the AVFrame * were somewhere
>> else than the beginning of SVQ3Frame.)
>>
>> - Andreas
>>
> I just noticed that avcodec_open2() will call the codec's close function
> even when init has never been called when the FF_CODEC_CAP_INIT_CLEANUP
> flag is set. This is against the documentation of said flag and it is
> also complete nonsense. Will send a patch for this.
> 
Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-September/270336.html

- Andreas