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

[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.

- Andreas