tor 2024-03-28 klockan 03:48 +0100 skrev Michael Niedermayer: > On Wed, Mar 27, 2024 at 11:13:48AM +0100, Tomas Härdin wrote: > > mån 2024-03-25 klockan 21:04 +0100 skrev 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 > > > > > > > > > <mich...@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 > > > > That all calls get_qcx(). > > yes > > > > If you git grep for nguardbits you'll see > > it's only ever set there when decoding, and similarly with expn. > > yes > > > > Coding > > style and quantization style are not the same thing. > > yes > > > but still, you can try to add a check the values for both nguardbits > and expn which lead to undefined shifts in ff_jpeg2000_decode_htj2k() > are used in normal jpeg2000 and break these samples
Hum hum.. You seem to be speaking of cblk->nonzerobits which is used in decode_cblk() for the Part 1 decoder. It only accepts bpno in 0..29. bpno in turn depends on roi_shift which is never negative. bpno = expn + nguardbits + roi_shift - 1 - zbp But zbp can be up to 100 if I understand correctly, so expn + guardbits can be up to 130 and still be legal Part 1. So yeah, putting the check in get_qcx() isn't right. But it could live in tile_codeblocks() in the bandno loop. As a bonus we only need to compute magp once per subband (the compiler probably already notices this). Interestingly roi_shift is sent to ff_jpeg2000_decode_htj2k() but never used. T.814 seems to indicate ROI is still used for HTJ2K so maybe we should warn if it's non-zero? /Tomas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".