Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/cbs_av1: Avoid shift overflow

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2398 bytes --]

On Thu, May 09, 2024 at 09:38:14PM +0100, Mark Thompson wrote:
> On 09/05/2024 16:10, Michael Niedermayer wrote:
> > ffmpeg | branch: master | Michael Niedermayer <michael@niedermayer.cc> | Wed May  1 21:44:33 2024 +0200| [d7924a4f60f2088de1e6790345caba929eb97030] | committer: Michael Niedermayer
> > 
> > avcodec/cbs_av1: Avoid shift overflow
> > 
> > Fixes: CID1465488 Unintentional integer overflow
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > 
> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=d7924a4f60f2088de1e6790345caba929eb97030
> > ---
> > 
> >  libavcodec/cbs_av1.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> > index 1d9ac5ab44..fb82996022 100644
> > --- a/libavcodec/cbs_av1.c
> > +++ b/libavcodec/cbs_av1.c
> > @@ -301,7 +301,7 @@ static int cbs_av1_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
> >          return AVERROR(ENOSPC);
> >  
> >      if (len > 0)
> > -        put_bits(pbc, len, (1 << len) - 1 - (value != range_max));
> > +        put_bits(pbc, len, (1U << len) - 1 - (value != range_max));
> >  
> >      CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
> >  
> What syntax element can call this with range_max - range_min == 31?  (Do you have a stream?)

coverity is a tool doing static analysis. There are no streams.

cbs_av1_write_increment() checks its inputs at the begin of the function

    av_assert0(range_min <= range_max && range_max - range_min < 32);
    if (value < range_min || value > range_max) ...
        "error"

If we assume these are correct then the function is supposed to support
range_max = range_min + 31
otherwise the assert really should be range_max - range_min < 31
both value = range_max and value = range_max - 1 will overflow
1 << len but not 1U << len

Do you suggest that the range in the assert() is wrong ?

The patch/commit intended to fix the discrepancy of the asserts range
and what the function actually supports. Iam not sure if an input stream
can trigger this.

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".