[FFmpeg-devel] [PATCH 1/4] avcodec/jpeg2000htdec: Check M_b / magp before using it in a shift

[Michael Niedermayer]

On Mon, Mar 25, 2024 at 08:13:13PM +0100, Michael Niedermayer wrote:
> On Thu, Mar 21, 2024 at 04:07:14PM +0100, Tomas Härdin wrote:
> > ons 2024-03-20 klockan 21:35 +0100 skrev Tomas Härdin:
> > > ons 2024-03-20 klockan 14:12 +0100 skrev Michael Niedermayer:
> > > > On Wed, Mar 20, 2024 at 12:20:11PM +0100, Tomas Härdin wrote:
> > > > > ons 2024-03-20 klockan 03:59 +0100 skrev Michael Niedermayer:
> > > > > > Fixes: shift exponent -1 is negative
> > > > > > Fixes: 65378/clusterfuzz-testcase-minimized-
> > > > > > ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5457678193197056
> > > > > > 
> > > > > > Found-by: continuous fuzzing process
> > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > > > > ---
> > > > > >  libavcodec/jpeg2000htdec.c | 3 +++
> > > > > >  1 file changed, 3 insertions(+)
> > > > > > 
> > > > > > diff --git a/libavcodec/jpeg2000htdec.c
> > > > > > b/libavcodec/jpeg2000htdec.c
> > > > > > index 6b9898d3ff..0b94bb5da2 100644
> > > > > > --- a/libavcodec/jpeg2000htdec.c
> > > > > > +++ b/libavcodec/jpeg2000htdec.c
> > > > > > @@ -1193,6 +1193,9 @@ ff_jpeg2000_decode_htj2k(const
> > > > > > Jpeg2000DecoderContext *s, Jpeg2000CodingStyle *c
> > > > > >  
> > > > > >      int32_t M_b = magp;
> > > > > >  
> > > > > > +    if (magp >= 31)
> > > > > > +        return AVERROR_INVALIDDATA;
> > > > > 
> > > > > This isn't where the error is, assuming it even is an error. It's
> > > > > either expn or nguardbits that are wrong, and they should be
> > > > > detected
> > > > > and reported as such in jpeg2000dec.c. Checking this in every
> > > > > call
> > > > > to
> > > > > ff_jpeg2000_decode_htj2k() is wasteful.
> > > > > 
> > > > > nguardbits can be 0..7 and expn can be 0..31. Table A.11
> > > > > indicates
> > > > > that
> > > > > Ssize can be up to 38 bits, so M_b >= 31 is in fact perfectly
> > > > > valid.
> > > > 
> > > > > A
> > > > > more appropriate error might be AVERROR_PATCHWELCOME.
> > > > 
> > > > indeed, i will change it to AVERROR_PATCHWELCOME
> > > 
> > > Please also move it further up so as to not waste cycles checking it
> > > every time
> > 
> > To be more precise, get_qcx() looks like the proper place for it
> 
> will apply with teh check moved there

the values that are causing undefined behavior for htj2k are used in normal
j2k knowing which type of j2k we have seems decided by COC/COD/COX

so i dont think we can check in QCX, because a later COX could
make it both invalid or valid
and we cannot check in COX as a later QCX can similarly change it

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- 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/20240325/e0259e91/attachment.sig>