On Sat, May 18, 2024 at 08:02:28AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Helps: CID1441939 Unchecked return value
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/tiff.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> > index ca7e9f6aba9..31de6ad7308 100644
> > --- a/libavcodec/tiff.c
> > +++ b/libavcodec/tiff.c
> > @@ -457,7 +457,8 @@ static void unpack_gray(TiffContext *s, AVFrame *p,
> >      GetBitContext gb;
> >      uint16_t *dst = (uint16_t *)(p->data[0] + lnum * p->linesize[0]);
> >  
> > -    init_get_bits8(&gb, src, width);
> > +    int ret = init_get_bits8(&gb, src, width);
> > +    av_assert1(ret >= 0);
> >  
> >      for (int i = 0; i < s->width; i++) {
> >          dst[i] = get_bits(&gb, bpp);
> 
> What guarantees that this is not triggered?

Several arguments, first one is simply that linesize*allocated_height must be addressable with an int index
which in practice ends on the check "stride*(uint64_t)(h+128) >= INT_MAX" in av_image_check_size2

so I would expect a width * 8 not to overflow if a stride * (h+128) cannot
(this is a bit fuzzy as our width can contain some subsampling factors though i
doubt they can be that large)

the 2nd is that
int width = ((s->width * s->bpp) + 7) >> 3;
or teh alethernative path contains a av_assert0(width <= bytes_per_row);
where int bytes_per_row = (((s->width - 1) / s->subsampling[0] + 1) * s->bpp *
                            s->subsampling[0] * s->subsampling[1] + 7) >> 3;
both are integers divided by 8 so i would expect no overflow on a multiply by 8

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides