[FFmpeg-devel] [PATCH 1/3] avcodec/cbs_jpeg: Assert that cbs_jpeg_assemble_fragment() stays within the array

[Michael Niedermayer]

On Wed, May 08, 2024 at 11:46:45AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Inspired by: CID1473561 Untrusted pointer write
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/cbs_jpeg.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> > index b1b58dcd65e..628841c5f37 100644
> > --- a/libavcodec/cbs_jpeg.c
> > +++ b/libavcodec/cbs_jpeg.c
> > @@ -401,6 +401,7 @@ static int cbs_jpeg_assemble_fragment(CodedBitstreamContext *ctx,
> >                  } else {
> >                      data[dp++] = unit->data[sp];
> >                  }
> > +                av_assert0(dp <= size - 2);
> >              }
> >          }
> >      }
> 
> You want to add an av_assert0 to a hot loop (or rather: to what would be
> a hot loop in case this code were executed) just because Coverity thinks
> that reading data with a different endianness taints this data? (That
> the supposedly tainted variable has actually already been checked via an
> assert makes this even more crazy.)

patch droped

But the code is fragile, there are 2 loops that must match exactly
if what the second writes doesnt match what the first counts it writes
out of array
This just needs someone finding a bug in the loop and fix it without
updating the 2nd loop

thx

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

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240508/a4b48210/attachment.sig>