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 369BB4534E for ; Mon, 25 Dec 2023 02:05:38 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D003B68D169; Mon, 25 Dec 2023 04:05:35 +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 58D8E68CFB7 for ; Mon, 25 Dec 2023 04:05:29 +0200 (EET) Received: by mail.gandi.net (Postfix) with ESMTPSA id 4811660004 for ; Mon, 25 Dec 2023 02:05:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1703469928; 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=AN2VwDDTHQT2tgYAbQdV0QNdT2PIqsLC8GI5fa22cyU=; b=RxYxbREcyaP5X9by7rMF80A6oCDURSIVRmGEnKWgMk6DbRs+puHS1jAZ/+YSHzC6bBcpFl eHbq5a/OypYfsVcKxd6G0yi4Rm6A5lH0uI7JKiiuJuD1Y28UazdYvwwL8NJROPT9DbFGhY p50QjZDsvg99HQtRGHYgOgYHSlIRmlIN+c9gdFgJy/rrrktUDxaPENbwSfmFoeSmGh/WRm Ie322nDed4729kWbfIWdTdMUIVspSOxd/bnAsryHYJ7By2UWKWZ0MQpqgdgzXCp7HO1gbz KFWtl+LFh0Xndv1GoxlhJGoeqAp1+jvtrlJMJsiVhHxldfIHksID803asymZvw== Date: Mon, 25 Dec 2023 03:05:27 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20231225020527.GN6420@pb2> References: <20231223025733.85366-1-leo.izen@gmail.com> MIME-Version: 1.0 In-Reply-To: <20231223025733.85366-1-leo.izen@gmail.com> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH] avcodec/jpegxl_parser: check ANS cluster alphabet size vs bundle size 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="===============1206443868098645390==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============1206443868098645390== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0oEQci8Wf3TZ4Qnh" Content-Disposition: inline --0oEQci8Wf3TZ4Qnh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 22, 2023 at 09:57:33PM -0500, Leo Izen wrote: > The specification doesn't mention that clusters cannot have alphabet > sizes greater than 1 << bundle->log_alphabet_size, but the reference > implementation rejects these entropy streams as invalid, so we should > too. Refusing to do so can overflow a stack variable on line 556 that > should be large enough otherwise. >=20 > Fixes #10738. >=20 > Reported-by: Michael Niedermayer The issue has been discovered by Zeng Yunxiang and Li Zeyuan. as mentioned = in the ticket > Signed-off-by: Leo Izen > --- > libavcodec/jpegxl_parser.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) >=20 > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c > index 006eb6b295..c9832e4393 100644 > --- a/libavcodec/jpegxl_parser.c > +++ b/libavcodec/jpegxl_parser.c > @@ -388,7 +388,6 @@ static int populate_distribution(GetBitContext *gb, J= XLSymbolDistribution *dist, > =20 > if (get_bits1(gb)) { > /* simple code */ > - dist->alphabet_size =3D 256; > if (get_bits1(gb)) { > uint8_t v1 =3D jxl_u8(gb); > uint8_t v2 =3D jxl_u8(gb); > @@ -398,10 +397,12 @@ static int populate_distribution(GetBitContext *gb,= JXLSymbolDistribution *dist, > dist->freq[v2] =3D (1 << 12) - dist->freq[v1]; > if (!dist->freq[v1]) > dist->uniq_pos =3D v2; > + dist->alphabet_size =3D 1 + FFMAX(v1, v2); > } else { > uint8_t x =3D jxl_u8(gb); > dist->freq[x] =3D 1 << 12; > dist->uniq_pos =3D x; > + dist->alphabet_size=3D 1 + x; > } > return 0; > } > @@ -880,6 +881,8 @@ static int read_distribution_bundle(GetBitContext *gb= , JXLEntropyDecoder *dec, > ret =3D populate_distribution(gb, &bundle->dists[i], bundle-= >log_alphabet_size); > if (ret < 0) > return ret; > + if (bundle->dists[i].alphabet_size > (1 << bundle->log_alpha= bet_size)) > + return AVERROR_INVALIDDATA; i think alphabet_size should be checked before it is stored in the struct or at least before it is used. ATM the value is unchecked and substantial processing is done with it in populate_distribution() before this check Also log_alphabet_size for use_prefix_code =3D=3D 0 is limited to a max of 8 which limits alphabet_size to 256 in that codepath with the new check. There are also various arrays that can be reduced in size when alphabet_size is limited in this codepath. But thats for a different time and patch. For now i think just moving the alphabet_size check, is fine thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle --0oEQci8Wf3TZ4Qnh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZYjjYQAKCRBhHseHBAsP q6JuAJ9PuDdmkA+3EIcet7xKBM6LS2PpSACfSJSGDS6JitqhgXrBO7xewYtvGjQ= =/2H6 -----END PGP SIGNATURE----- --0oEQci8Wf3TZ4Qnh-- --===============1206443868098645390== 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". --===============1206443868098645390==--