From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 39D0E4A435 for ; Thu, 28 Mar 2024 02:49:06 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8E18768D5F2; Thu, 28 Mar 2024 04:49:04 +0200 (EET) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5118768D24D for ; Thu, 28 Mar 2024 04:48:58 +0200 (EET) Received: by mail.gandi.net (Postfix) with ESMTPSA id 8A07360002 for ; Thu, 28 Mar 2024 02:48:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1711594137; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=aWxpsDlmyaqch2+BBmjRwNV57v8dFurf865OyCS+jPE=; b=fYUhcEeB5KKHID8EOqjyWznivfAtzq80bc/1JZJj1RA7robiNnoNEU41ad3DBkWM3sXHSv XrvMaaZaG1Xa5zrkgpspqqXa1PwJxKX9Gqp/AeW3LPLoPrIXdKhoKsdtFq7lPCpyUFvB/L tGhOo+Pnlwfw3fkcLccgiLbMK9pqe3f6G8AP91Qz78Ts/m8tEmRNIItXtHEp16y6Wn0iBc 9WwRp9NQnBsdV3NZvKTYQakK+ODS5gEaG8+ltKinv4SmGnmp1nIiJWXFRqoUs29LH3rr3Z qUfefAezutHNCIfKLdFPbIZoOQFoMqQz06thBclU/NXNEYbwhxyLLqBtrsRseQ== Date: Thu, 28 Mar 2024 03:48:56 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240328024856.GN6420@pb2> References: <20240320025923.3794-1-michael@niedermayer.cc> <092a8cd44e3074e2d0462052e715281c08eb31b9.camel@haerdin.se> <20240320131216.GR6420@pb2> <0e1355913d2f11aa14a712e1ea9a96781c10d290.camel@haerdin.se> <8f924f3320dd21fd5d41872c46566c6003f392bd.camel@haerdin.se> <20240325191313.GO6420@pb2> <20240325200424.GQ6420@pb2> <0892db70a5af069d8f02e50f3994ed8ebe886c5d.camel@haerdin.se> MIME-Version: 1.0 In-Reply-To: <0892db70a5af069d8f02e50f3994ed8ebe886c5d.camel@haerdin.se> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH 1/4] avcodec/jpeg2000htdec: Check M_b / magp before using it in a shift X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============0673328315726357456==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============0673328315726357456== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="I4qmv6OVPnpd7fz+" Content-Disposition: inline --I4qmv6OVPnpd7fz+ Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 27, 2024 at 11:13:48AM +0100, Tomas H=E4rdin wrote: > m=E5n 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=E4rdin wrote: > > > > ons 2024-03-20 klockan 21:35 +0100 skrev Tomas H=E4rdin: > > > > > ons 2024-03-20 klockan 14:12 +0100 skrev Michael Niedermayer: > > > > > > On Wed, Mar 20, 2024 at 12:20:11PM +0100, Tomas H=E4rdin 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 > > > > > > > >=20 > > > > > > > > Found-by: continuous fuzzing process > > > > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffm= peg > > > > > > > > Signed-off-by: Michael Niedermayer > > > > > > > > > > > > > > > > --- > > > > > > > > =A0libavcodec/jpeg2000htdec.c | 3 +++ > > > > > > > > =A01 file changed, 3 insertions(+) > > > > > > > >=20 > > > > > > > > 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 > > > > > > > > =A0 > > > > > > > > =A0=A0=A0=A0 int32_t M_b =3D magp; > > > > > > > > =A0 > > > > > > > > +=A0=A0=A0 if (magp >=3D 31) > > > > > > > > +=A0=A0=A0=A0=A0=A0=A0 return AVERROR_INVALIDDATA; > > > > > > >=20 > > > > > > > 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. > > > > > > >=20 > > > > > > > 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 >=3D 31 is in fact > > > > > > > perfectly > > > > > > > valid. > > > > > >=20 > > > > > > > A > > > > > > > more appropriate error might be AVERROR_PATCHWELCOME. > > > > > >=20 > > > > > > indeed, i will change it to AVERROR_PATCHWELCOME > > > > >=20 > > > > > Please also move it further up so as to not waste cycles > > > > > checking it > > > > > every time > > > >=20 > > > > To be more precise, get_qcx() looks like the proper place for it > > >=20 > > > will apply with teh check moved there > >=20 > > 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 > >=20 > > 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 >=20 > 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 [...] --=20 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. --I4qmv6OVPnpd7fz+ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZgTakgAKCRBhHseHBAsP q8cLAJ0fVFCF63osUMHRPztjK3qkIuOfdACdFkZjTHpLKry0s/LK0K+mAqNjzjw= =/PfB -----END PGP SIGNATURE----- --I4qmv6OVPnpd7fz+-- --===============0673328315726357456== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============0673328315726357456==--