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

[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