[FFmpeg-devel] [PATCH 3/3] avcodec/cbs_h265_syntax_template: Limit num_long_term_pics more strictly

[James Almer]

On 4/20/2020 7:03 PM, Michael Niedermayer wrote:
> The limit is based on hevcdec.c
> Fixes: 20854/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_METADATA_fuzzer-5160442882424832
> Fixes: out of array access
> 
> 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, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index 180a045c34..b74b9648c3 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1367,7 +1367,7 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
>                      infer(num_long_term_sps, 0);
>                      idx_size = 0;
>                  }
> -                ue(num_long_term_pics, 0, HEVC_MAX_LONG_TERM_REF_PICS);
> +                ue(num_long_term_pics, 0, FFMIN(HEVC_MAX_LONG_TERM_REF_PICS, FF_ARRAY_ELEMS(current->poc_lsb_lt) - current->num_long_term_sps));

Maybe poc_lsb_lt should also have HEVC_MAX_LONG_TERM_REF_PICS elems
instead of HEVC_MAX_REFS, same as in the hevc decoder.

Also the spec defines some specific limit to this field:

"When nuh_layer_id is equal to 0, the value of num_long_term_pics shall
be less than or equal to sps_max_dec_pic_buffering_minus1[TemporalId] −
NumNegativePics[CurrRpsIdx] − NumPositivePics[CurrRpsIdx] −
num_long_term_sps − TwoVersionsOfCurrDecPicFlag"

With CurrRpsIdx derived as:
– If short_term_ref_pic_set_sps_flag is equal to 1, CurrRpsIdx is set
equal to short_term_ref_pic_set_idx.
– Otherwise, CurrRpsIdx is set equal to num_short_term_ref_pic_sets.

And TwoVersionsOfCurrDecPicFlag as:
"TwoVersionsOfCurrDecPicFlag = pps_curr_pic_ref_enabled_flag &&
(sample_adaptive_offset_enabled_flag ||
!pps_deblocking_filter_disabled_flag ||
deblocking_filter_override_enabled_flag)
When sps_max_dec_pic_buffering_minus1[ TemporalId ] is equal to 0, the
value of TwoVersionsOfCurrDecPicFlag shall be equal to 0."

But i don't know if it's worth adding that many checks.