[FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the bitstream position check

[Dai, Jianhui J]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Monday, March 18, 2024 7:35 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Improve the
> bitstream position check
> 
> Dai, Jianhui J:
> > The VP8 compressed header may not be byte-aligned due to boolean
> > coding. Use bitwise comparison to prevent the potential overread.
> >
> > Signed-off-by: Jianhui Dai <jianhui.j.dai at intel.com>
> > ---
> >  libavcodec/cbs_vp8.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index
> > 065156c248..13acad3724 100644
> > --- a/libavcodec/cbs_vp8.c
> > +++ b/libavcodec/cbs_vp8.c
> > @@ -327,9 +327,10 @@ static int cbs_vp8_read_unit(CodedBitstreamContext
> *ctx,
> >      if (err < 0)
> >          return err;
> >
> > +    // Position may not be byte-aligned after compressed header; using bits
> > +    // count comparison for accuracy.
> >      pos = get_bits_count(&gbc);
> > -    pos /= 8;
> > -    av_assert0(pos <= unit->data_size);
> > +    av_assert0(pos <= unit->data_size * 8);
> 
> (pos + 7U) / 8 seems better to avoid potential overflow issues (not an issue atm,
> but if we ever were to use e.g. 64bit for bitcount of the GetBit API, then the
> multiplication on the right could overflow a 32bit size_t).

Thanks. Fixed it in PATCH v2. Please take a look.

> 
> >
> >      frame->data_ref = av_buffer_ref(unit->data_ref);
> >      if (!frame->data_ref)
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
> with subject "unsubscribe".