[FFmpeg-devel] [PATCH 1/2] avcodec/cbs_h265_syntax_template: Better check for num_long_term_sps

[Mark Thompson]

On 14/11/2020 10:18, Michael Niedermayer wrote:
> Fixes: index 26 out of bounds for type 'uint8_t [16]'
> Fixes: 24913/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_METADATA_fuzzer-6261760693370880
> 
> 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_h265_syntax_template.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index 48fae82d04..8eb6e159f4 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1405,6 +1405,8 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
>                       infer(num_long_term_sps, 0);
>                       idx_size = 0;
>                   }
> +                if (HEVC_MAX_REFS < current->num_long_term_sps)
> +                    return AVERROR_INVALIDDATA;

Please don't put isolated tests in the middle of the template.  If it constrains a value then add it to the constraints on that value.

>                   ue(num_long_term_pics, 0, HEVC_MAX_REFS - current->num_long_term_sps);
>   
>                   for (i = 0; i < current->num_long_term_sps +
> 

It would be good if the commit message could include an explanation derived from the standard, too.

As far as I can tell the constraint doesn't appear explicitly, but the SPS is allowed to define more possible long term frames than are actually allowed to be present at any given moment so we need the tighter bound.

- Mark