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 D1C4648F8F for ; Sun, 30 Jun 2024 23:07:14 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D16CF68D265; Mon, 1 Jul 2024 02:07:10 +0300 (EEST) Received: from mail-pg1-f171.google.com (mail-pg1-f171.google.com [209.85.215.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0957468BDE1 for ; Mon, 1 Jul 2024 02:07:04 +0300 (EEST) Received: by mail-pg1-f171.google.com with SMTP id 41be03b00d2f7-71816f36d4dso1648705a12.2 for ; Sun, 30 Jun 2024 16:07:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719788821; x=1720393621; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=XMdb3sKTUvV/o7E+hAreF7+yWWQb/mbP8IxdUWs3Uo4=; b=JEFsEgyfFqL6qK0NQTyU+nrVBqn3t1tBDgObrNJWmtOwPLR8d0U+yG3EL3Bwk0avp5 /gFx5iQ07ZAT0s2SMQJM1aqYvfCYuLSV56U4eKS6mWRa9S4+z9Pz+2Sa8nbWy1wT8k7W kFV06BP2pva4gY7hGe6ZXnDUhwk2uGf8f1TSbohG6kxFZ0fmidgFXeS+xFsmXKmNTVth qrpc+OvaKkwD+aTZvBZfqDPIUmsEntmDTzvU4HUiwGpXbOlgw4w4FRPzPOI/Pes9VshA bz8JNmBlDGA0XQZsDy0Eqpqqkew3duRMvy+b49gTxs2NxbuuBCH/AqsMVsbgNLVkrn89 x7QQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719788821; x=1720393621; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=XMdb3sKTUvV/o7E+hAreF7+yWWQb/mbP8IxdUWs3Uo4=; b=CEFoKx9HiVurJcypBFR/JtpSfgAds0JXa1FE9FlqkrRfB9JaPlCNAoyP46HkCuT2dq KRiJp0zPxRQdo0e4GuOV+DyK6vvB1Aktsef8aUrTj8YrlE3tzFLTBu9Mx3AMBc+X/ISC HWkLyWjqkDuu+fwPYqck1X1N0YAXy+4LoZMVfblQaXH2TueK9J7A4But1wfsOYFeituv zhMQiUo4AUiaFrLwlCXmVDcsJUq2Sv4IrFBMungrODv/mzgbL7UqBG9bpJcKehxLra0Y bvmV4qZymdkyvhFTBOHl4OFuJOnUO4KicpL/69k5QDhWzP/KKzJR34DcUyWdBQ5wRLma vAow== X-Gm-Message-State: AOJu0Yxb5exPmjBnZHtEurICP2nLNTdOLmItZDvVPvB+TJj/GuEPUQrm 8n/I5vJdhXCakWgSRW4MWXYvRnocrQyfhdVqdGRMy+n5vUBiYPQ4ywfZgA== X-Google-Smtp-Source: AGHT+IGSOjh+1fv7XdzWafousYwJGsNFK9KpeME6+4MGVgYjbfAe+IG4GOzWOuf10aK3vbe1GzxvWw== X-Received: by 2002:a05:6a20:2590:b0:1bd:1a06:7ef2 with SMTP id adf61e73a8af0-1bef610ab4bmr8059685637.25.1719788820519; Sun, 30 Jun 2024 16:07:00 -0700 (PDT) Received: from [192.168.0.16] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-708043b703fsm5324605b3a.145.2024.06.30.16.06.59 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 30 Jun 2024 16:06:59 -0700 (PDT) Message-ID: <527948e0-a335-4ad8-87fa-0a3443b32203@gmail.com> Date: Sun, 30 Jun 2024 20:07:28 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240322230818.18997-1-michael@niedermayer.cc> <40908cf3-7e3a-4a57-a23e-43bf153c20bd@gmail.com> <20240629233705.GB4991@pb2> Content-Language: en-US From: James Almer In-Reply-To: <20240629233705.GB4991@pb2> 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 6/29/2024 8:37 PM, Michael Niedermayer wrote: > On Wed, Jun 26, 2024 at 09:52:44PM -0300, James Almer wrote: >> On 3/22/2024 8:08 PM, Michael Niedermayer wrote: >>> 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 || >>> + st->codecpar->bits_per_coded_sample > 64) >> >> Where does the process take so long that oss-fuzz gets a timeout if these >> are unreasonably high? I don't see nb_channels used anywhere in here where >> that matters. >> Is it in the PCM decoder? Because that decoder is meant to handle any >> arbitrary amount of channels, so limiting it to whatever FF_SANE_NB_CHANNELS >> is set to is not ok. > > libavutil/channel_layout.c:627:17 > or it will OOM before depending on how much memory is available Does this fix the timeout? > diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c > index 2d6963b6df..a4fcd199e1 100644 > --- a/libavutil/channel_layout.c > +++ b/libavutil/channel_layout.c > @@ -623,6 +623,8 @@ int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout, > for (i = 0; i < channel_layout->nb_channels; i++) { > enum AVChannel ch = av_channel_layout_channel_from_index(channel_layout, i); > > + if (!av_bprint_is_complete(bp)) > + break; > if (i) > av_bprintf(bp, "+"); > av_channel_name_bprint(bp, ch); But this is not ok as it will affect the buffer len value av_channel_layout_describe() returns on success when truncation took place, so something else would have to be done. > > >> >> And is the bits_per_coded_sample > 64 check to prevent codec_id being >> AV_CODEC_ID_NONE? if so, how does that affect demuxing time? >> AV_CODEC_ID_NONE for that matter could happen for valid files with a codec >> we don't currently support. > > We generally check read values for validity at the earliest, > bits_per_coded_sample of more than 64 seem not valid to me. I agree the check is fine, but my point is that, assuming this is affecting demuxing time, this check as is may be hiding an issue related to codec_id being set to AV_CODEC_ID_NONE here (the result of ff_get_pcm_codec_id() with an unsupported bits_per_coded_sample value), so it should be looked at more closely because said codec_id could happen with valid files using codecs the demuxer does not know about. If it does not affect demuxing time and is a "just in case" check, then it doesn't belong in a patch that says "Fixes: Timeout" and mentions an ossfuzz issue. > > thx > > [...] > > > _______________________________________________ > 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". _______________________________________________ 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".