[FFmpeg-devel] [PATCH 05/13] avcodec/cbs_av1_syntax_template: Check ref_frame_idx before use

[James Almer]

On 8/4/2019 1:44 PM, Michael Niedermayer wrote:
> Fixes: index -1 out of bounds for type 'AV1ReferenceFrameState [8]'
> Fixes: 16079/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5758807440883712
> 
> 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/cbs_av1_syntax_template.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index b04cd51d55..806b302de6 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -419,16 +419,17 @@ static int FUNC(frame_size_with_refs)(CodedBitstreamContext *ctx, RWContext *rw,
>      for (i = 0; i < AV1_REFS_PER_FRAME; i++) {
>          flags(found_ref[i], 1, i);
>          if (current->found_ref[i]) {
> -            AV1ReferenceFrameState *ref =
> -                &priv->ref[current->ref_frame_idx[i]];
> +            AV1ReferenceFrameState *ref;
>  
> -            if (!ref->valid) {
> +            if (current->ref_frame_idx[i] < 0 ||
> +                !priv->ref[current->ref_frame_idx[i]].valid) {
>                  av_log(ctx->log_ctx, AV_LOG_ERROR,
>                         "Missing reference frame needed for frame size "
>                         "(ref = %d, ref_frame_idx = %d).\n",
>                         i, current->ref_frame_idx[i]);
>                  return AVERROR_INVALIDDATA;
>              }
> +            ref = &priv->ref[current->ref_frame_idx[i]];
>  
>              priv->upscaled_width = ref->upscaled_width;
>              priv->frame_width    = ref->frame_width;

This actually revealed a bug when setting ref_frame_idx[i] in the
frame_refs_short_signaling == true code path. It's incomplete given that
the -1 is a placeholder meant to be replaced further into the process.

This change is ok to prevent the out of bounds issue for now, but valid
files are in theory being rejected, and that should be fixed.