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 9939540948 for ; Sun, 3 Jul 2022 10:00:42 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0FEF768B972; Sun, 3 Jul 2022 13:00:39 +0300 (EEST) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E7C1A68B95E for ; Sun, 3 Jul 2022 13:00:32 +0300 (EEST) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 263A0Wpp030397 for ; Sun, 3 Jul 2022 12:00:32 +0200 Received: by phare.normalesup.org (Postfix, from userid 1001) id 0A5EBE00E9; Sun, 3 Jul 2022 12:00:32 +0200 (CEST) Date: Sun, 3 Jul 2022 12:00:31 +0200 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: <20220702222532.9609-1-jamrial@gmail.com> MIME-Version: 1.0 In-Reply-To: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Sun, 03 Jul 2022 12:00:32 +0200 (CEST) Subject: Re: [FFmpeg-devel] [PATCH] avutil/channel_layout: don't error out on truncated strings 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="===============6172065193810817877==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============6172065193810817877== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Cz9kkbIAHL6OKfN4" Content-Disposition: inline --Cz9kkbIAHL6OKfN4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Andreas Rheinhardt (12022-07-03): > > if (!av_bprint_is_complete(bp)) > > - return AVERROR(ENOMEM); > > + break; > > =20 > Isn't this actually still against the API? av_channel_layout_describe() > will not return the correct number of bytes necessary to write the > string for the channel layout. You are both right. BPrint-based APIs are not supposed to check for truncation, because printing into a bounded buffer to determine the necessary size is a valid use (see AV_BPRINT_SIZE_COUNT_ONLY). What is wrong is Michael's original fix: >> commit 8154cb7c2ff2afcb1a0842de8c215b7714c814d0 >> Author: Michael Niedermayer >> Date: 2022-06-30 00:00:32 +0200 >>=20 >> avutil/channel_layout: av_channel_layout_describe_bprint: Check for = buffer end >>=20 >> Fixes: Timeout printing a billion channels >> Fixes: 48099/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzz= er-6754782204788736 >>=20 >> Found-by: continuous fuzzing process https://github.com/google/oss-f= uzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer >>=20 >> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c >> index 21b70173b7..1887987789 100644 >> --- a/libavutil/channel_layout.c >> +++ b/libavutil/channel_layout.c >> @@ -757,6 +757,10 @@ int av_channel_layout_describe_bprint(const AVChann= elLayout *channel_layout, >> if (channel_layout->order =3D=3D AV_CHANNEL_ORDER_CUSTOM && >> channel_layout->u.map[i].name[0]) If the channel count is insanely high, then this will cause invalid reads. >> av_bprintf(bp, "@%s", channel_layout->u.map[i].name); >> + >> + if (!av_bprint_is_complete(bp)) >> + return AVERROR(ENOMEM); >> + >> } >> if (channel_layout->nb_channels) { >> av_bprintf(bp, ")"); Obviously, this fuzzer found a case where a demuxer or a decoder constructs an invalid channel layout in memory without proper validation. There is a bug, possibly an security-related one, and this only hides it from the test suite. The proper fix is to find where the input is improperly validated, and then revert this change. I do not know how to exploit the "48099/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6754782204= 788736" information to reproduce the bug and investigate. Regards, --=20 Nicolas George --Cz9kkbIAHL6OKfN4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE6ooRQGBoNzw0KnwPcZVLI8pNxgwFAmLBaL4ACgkQcZVLI8pN xgw+jQ/+Mfxedc7dRws/kRYmN50hVz8bcDtYgyZiulNn5r80pyaiKrQ3Ln+NBtwt uD+veT2hhcUC+POAcnpPOnCJH/ofgxjKahK17GsIqrkwhLY5JFF8+rOOE5s8GCu3 DnPOQdm1ZQzlfk6ZBwJnxMcziU5IflgOYkGw93DHRAGgCaaXFYjxNpG/5l6LVmtS LDF3y+1N3kkLp35sfyzoJ5U1uhLxlFLZ6jTaMP3aPL84k765aZJ1IlmPgRJLE8sq 1CMfCtlbRHOH7gW2er94LijJtEMqn/b9VLnLb62f1UEmMELKmSZUIAFMJmR39RR/ do2Rqgrj0M2XC4+3Jqxqmd/gHlMWhX5LfNOxxVgUErKu4QKwc4MJYij8rzAaCbwa REuKgDkPj6s26zLl/xa+dm0T/g6KWkiZsGGKbGr5ENB0qY6nFBbWfs+KtHnce5kA 6xAcGcElH1hY4poUGvnrAMcdvkwei2E7OpsffOiqnZFKEOtL9d22Dgdtpkc37xcq dBdLqlw8wQ6HinpH8Skg01IgFfiKI2/YojrRtCnSvCPEkJppQSavUdXFB+5AGz8Y FSH/oRaIXCeFQ43PdHDOlUHfyHkarPcslfoD9RMijGYRF9Hp6EpHVE7QheNmGmUw c0NKGH+OQXYSnvgsXnnvKORSbDTJ4cQqCfUlrkuqETXrwocVzF0= =X4c7 -----END PGP SIGNATURE----- --Cz9kkbIAHL6OKfN4-- --===============6172065193810817877== 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". --===============6172065193810817877==--