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 821394B0A3 for ; Thu, 27 Jun 2024 06:53:19 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D679B68D200; Thu, 27 Jun 2024 09:53:17 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7E3E868C7C2 for ; Thu, 27 Jun 2024 09:53:10 +0300 (EEST) Authentication-Results: mail0.khirnov.net; dkim=pass (2048-bit key; unprotected) header.d=khirnov.net header.i=@khirnov.net header.a=rsa-sha256 header.s=mail header.b=JagivlIe; dkim-atps=neutral Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id DCF48240DAC for ; Thu, 27 Jun 2024 08:53:09 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id NFki_6xjNf_u for ; Thu, 27 Jun 2024 08:53:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=khirnov.net; s=mail; t=1719471189; bh=UgoR3lbi6m3jK2/jIFUL2xAS0YFi5VRbPV4c4cqBVKQ=; h=Subject:From:To:In-Reply-To:References:Date:From; b=JagivlIecpKtpib4HaDDb+JD4kOof4bT3iDQLkA1eCU1SRhT5vfqYUc1PLYMjZCp3 j/BG1+LZ+EUxPwW+XfMA3tTLZ2iuOGstb+JlRNRXmnNCKPx2YDWC5K5fd5ATH1Sn9R 80p8xLfUU6wU8Pv3mY7ANHO3I2/8s+blDlQFtpsEp5h4vadqDYQFcKqwiqJyefe9FD Ta6yBzV3p0Po0qxkgxAQ9x3rZACTMNG8J+k+r2QTe6L31QafrP2R45GqYTOzztO+DO w/9Pfq1ujsfz+2/oiSwA440dPiHjgAQBdJYutxidteyuHwsOd8cVIa3VGbMcB8+hnE NLXv+l1QLx3CA== Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 40BE22404E5 for ; Thu, 27 Jun 2024 08:53:09 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id 2C21D1601B9; Thu, 27 Jun 2024 08:53:09 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20240626235012.GV4991@pb2> References: <20240322230818.18997-1-michael@niedermayer.cc> <171152515798.7287.8979021106267679923@lain.khirnov.net> <20240327232702.GM6420@pb2> <20240625192546.GG4991@pb2> <171934367519.21847.12248145136761974723@lain.khirnov.net> <20240626235012.GV4991@pb2> Mail-Followup-To: FFmpeg development discussions and patches Date: Thu, 27 Jun 2024 08:53:09 +0200 Message-ID: <171947118914.21847.15118547358198017606@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 1/3] avformat/cafdec: sanity check channels and bps 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: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Quoting Michael Niedermayer (2024-06-27 01:50:12) > On Tue, Jun 25, 2024 at 09:27:55PM +0200, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2024-06-25 21:25:46) > > > On Thu, Mar 28, 2024 at 12:27:02AM +0100, Michael Niedermayer wrote: > > > > On Wed, Mar 27, 2024 at 08:39:17AM +0100, Anton Khirnov wrote: > > > > > Quoting Michael Niedermayer (2024-03-23 00:08:16) > > > > > > Fixes: Timeout > > > > > > Fixes: 67044/clusterfuzz-testcase-minimized-ffmpeg_dem_CAF_fuzzer-5791144363491328 > > > > > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > > Signed-off-by: Michael Niedermayer > > > > > > --- > > > > > > libavformat/cafdec.c | 5 +++++ > > > > > > 1 file changed, 5 insertions(+) > > > > > > > > > > > > diff --git a/libavformat/cafdec.c b/libavformat/cafdec.c > > > > > > index 426c56b9bd..334077efb5 100644 > > > > > > --- a/libavformat/cafdec.c > > > > > > +++ b/libavformat/cafdec.c > > > > > > @@ -33,6 +33,7 @@ > > > > > > #include "isom.h" > > > > > > #include "mov_chan.h" > > > > > > #include "libavcodec/flac.h" > > > > > > +#include "libavcodec/internal.h" > > > > > > #include "libavutil/intreadwrite.h" > > > > > > #include "libavutil/intfloat.h" > > > > > > #include "libavutil/dict.h" > > > > > > @@ -87,6 +88,10 @@ static int read_desc_chunk(AVFormatContext *s) > > > > > > st->codecpar->ch_layout.nb_channels = avio_rb32(pb); > > > > > > st->codecpar->bits_per_coded_sample = avio_rb32(pb); > > > > > > > > > > > > + if (st->codecpar->ch_layout.nb_channels > FF_SANE_NB_CHANNELS || > > > > > > > > > > I dislike this. > > > > > > > > I dislike it too > > > > > > so what do we do about this ? > > > > About what? What is the actual problem that needs addressed? > > 67044/clusterfuzz-testcase-minimized-ffmpeg_dem_CAF_fuzzer-5791144363491328 > > > > > > > any objections to apply this ? > > > > yes, FF_SANE_NB_CHANNELS is a hack that should be removed, not spread > > a maximum number for each theoretically unlimited parameter is desirable I disagree. > This can be a user setable value or a compile time value when such is preferred. > > We can take this to the TC if you want. > > The same way as the number of files or number of bytes used by some cache > needs limits, Those limits are implemented at the OS level, not by individual programs. And they are runtime-configurable. > so do channels, and number of pixels. Does not follow. > One can remove them but users and companies concious about security and > efficiency with untrusted input in an (semi-) automated environment will likely > choose the codebase providing such features. [citation needed] My main objection to this approach is that you're addressing a symptom (fuzzer timeout) rather than the actual problem (some code scaling inappropriately with input size), and it's completely unclear where that actual problem even is, as cafdec does not seem to be doing anything with the channel count. That suggests the problem is really in some other code and you're just papering over it. -- Anton Khirnov _______________________________________________ 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".