Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h266_syntax_template: Check num_output_layers_in_ols

[Nuo Mi <nuomi2021@gmail.com>]

On Fri, Sep 22, 2023 at 12:04 AM James Almer <jamrial@gmail.com> wrote:

> On 9/19/2023 12:25 PM, Nuo Mi wrote:
> > from the specification:
> > For each OLS, there shall be at least one layer that is an output layer.
> In other words, for any value of i in the range of 0
> > to TotalNumOlss − 1, inclusive, the value of NumOutputLayersInOls[ i ]
> shall be greater than or equal to 1
> >
> > Fixes: index 257 out of bounds for type 'uint8_t [257]'
> > Fixes:
> 61160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-6709397181825024
> >
> > Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/cbs_h266_syntax_template.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> > index f0d428a478..2077aa63bf 100644
> > --- a/libavcodec/cbs_h266_syntax_template.c
> > +++ b/libavcodec/cbs_h266_syntax_template.c
> > @@ -891,6 +891,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
> RWContext *rw,
> >                       }
> >                   }
> >               }
> > +            if (!num_output_layers_in_ols[i])
>
> The only way for num_output_layers_in_ols[i] to be less than 1 is if
> none of the checks above succeeded, and since the array is stored on
> stack uninitialized, this to me looks like a potential use of
> uninitialized value.
>
Hi James,
Thank you for the review.
No. The issue triggered by the following condition.
1. current->vps_ols_mode_idc == 2
2. all current->vps_ols_output_layer_flag[i][k] is zero.
num_output_layers_in_ols[i] will be set to 0 in this case.

Actually,  num_output_layers_in_ols[i] will be set to a right value if
current->vps_ols_mode_idc equals 0, 1, 2, 4.
If current->vps_ols_mode_idc equals 3, it may have an unchecked read.
It's mainly because 3 is reserved by specification for future use, the
specification said we should ignore it.

>
> > +                return AVERROR_INVALIDDATA;
> >           }
> >           for (i = 1; i < total_num_olss; i++) {
> >               int num_layers_in_ols = 0;
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".