[FFmpeg-devel] [PATCH 1/2] avcodec/sanm: Check extradata_size before allocations

[Reimar Döffinger]

On 16.07.2019, at 00:50, Michael Niedermayer <michael at niedermayer.cc> wrote:

> Fixes: Leaks
> Fixes: 15349/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SANM_fuzzer-5102530557640704
> 
> 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/sanm.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
> index 25aee7220f..60e2f4c624 100644
> --- a/libavcodec/sanm.c
> +++ b/libavcodec/sanm.c
> @@ -491,6 +491,10 @@ static av_cold int decode_init(AVCodecContext *avctx)
> 
>     ctx->avctx   = avctx;
>     ctx->version = !avctx->extradata_size;
> +    if (!ctx->version && avctx->extradata_size < 1026) {
> +        av_log(avctx, AV_LOG_ERROR, "Not enough extradata.\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> 
>     avctx->pix_fmt = ctx->version ? AV_PIX_FMT_RGB565 : AV_PIX_FMT_PAL8;
> 
> @@ -506,11 +510,6 @@ static av_cold int decode_init(AVCodecContext *avctx)
>     if (!ctx->version) {
>         int i;
> 
> -        if (avctx->extradata_size < 1026) {
> -            av_log(avctx, AV_LOG_ERROR, "Not enough extradata.\n");
> -            return AVERROR_INVALIDDATA;
> -        }

This seems quite a bit less obvious.
Is that the only error return case, and is adding the cleanup code complex enough that this is the better choice?
Either way I'd recommend a comment like
// early sanity check before allocations to avoid need for deallocation code.