From: Michael Niedermayer <michael@niedermayer.cc> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH 1/4] avcodec/jpeg2000htdec: Check M_b / magp before using it in a shift Date: Thu, 28 Mar 2024 03:48:56 +0100 Message-ID: <20240328024856.GN6420@pb2> (raw) In-Reply-To: <0892db70a5af069d8f02e50f3994ed8ebe886c5d.camel@haerdin.se> [-- Attachment #1.1: Type: text/plain, Size: 4408 bytes --] 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 > > > > > > > > <michael@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 because the same get_qcx() is used both for "normal" jpeg2000 and htj2k so the check needs to know if its htj2k inside ff_jpeg2000_decode_htj2k() thats obvious, in get_qcx() its not one can use the coding style to tell them apart but thats not trivial with the various ways to slice and dice things and it might not be set when get_qcx() is run if iam missing something please tell me how to know in get_qcx() that teh data is only used for ht2jk (Its very possible iam missing something given i did not read the hundreads of pages of spec, and mostly assume that its as insane as possible) thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you think the mosad wants you dead since a long time then you are either wrong or dead since a long time. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2024-03-28 2:49 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-20 2:59 Michael Niedermayer 2024-03-20 2:59 ` [FFmpeg-devel] [PATCH 2/4] avcodec/vmixdec: Check shift before use Michael Niedermayer 2024-03-25 19:16 ` Michael Niedermayer 2024-03-20 2:59 ` [FFmpeg-devel] [PATCH 3/4] tools/target_dec_fuzzer: Adjust RKA threshold up further Michael Niedermayer 2024-03-20 2:59 ` [FFmpeg-devel] [PATCH 4/4] avformat/id3v2: read_uslt() check for the amount read Michael Niedermayer 2024-03-20 11:20 ` [FFmpeg-devel] [PATCH 1/4] avcodec/jpeg2000htdec: Check M_b / magp before using it in a shift Tomas Härdin 2024-03-20 13:12 ` Michael Niedermayer 2024-03-20 20:35 ` Tomas Härdin 2024-03-21 15:07 ` Tomas Härdin 2024-03-25 19:13 ` Michael Niedermayer 2024-03-25 20:04 ` Michael Niedermayer 2024-03-27 10:13 ` Tomas Härdin 2024-03-28 2:48 ` Michael Niedermayer [this message] 2024-03-28 11:13 ` Tomas Härdin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20240328024856.GN6420@pb2 \ --to=michael@niedermayer.cc \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git