diff mbox series

[FFmpeg-devel,2/9] avcodec/tiff: Assert init_get_bits8() success in unpack_gray()

Message ID 20240518035743.766552-2-michael@niedermayer.cc
State Accepted
Commit a287f17db22c0c85f0445a1d31139cc70e73205e
Headers show
Series [FFmpeg-devel,1/9] avcodec/tiff: Assert init_get_bits8() success in horizontal_fill() | expand

Checks

Context Check Description
andriy/make_fate_x86 success Make fate finished
yinshiyou/make_fate_loongarch64 success Make fate finished
yinshiyou/make_loongarch64 warning New warnings during build
andriy/make_x86 warning New warnings during build

Commit Message

Michael Niedermayer May 18, 2024, 3:57 a.m. UTC
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(-)

Comments

Andreas Rheinhardt May 18, 2024, 6:02 a.m. UTC | #1
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?

- Andreas
Michael Niedermayer May 18, 2024, 7:45 p.m. UTC | #2
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

[...]
diff mbox series

Patch

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);