From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <ffmpeg-devel-bounces@ffmpeg.org> Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 4AE2D40F5B for <ffmpegdev@gitmailbox.com>; Sat, 10 Aug 2024 15:34:19 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7299768D9BF; Sat, 10 Aug 2024 18:34:16 +0300 (EEST) Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0B73268D877 for <ffmpeg-devel@ffmpeg.org>; Sat, 10 Aug 2024 18:34:10 +0300 (EEST) Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-1fc65329979so28156745ad.0 for <ffmpeg-devel@ffmpeg.org>; Sat, 10 Aug 2024 08:34:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723304047; x=1723908847; 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=OTINblrLLv6TSGj30iqQEK49jLsnazYBVdiDP8hf71E=; b=SWybvVse7ftHsAyxX8wGD+iFCUBzVImfp+cu4BJCoWv3suOzLCKQ6pZkh/vGb0KvkR 1MgRfsh0lVrJOPWgrYYJfd4UegEOP3kGsI02XJJqicCqIqayzBH65mIaxlTzRas/sFhq DiD5G7lZCAiukgP2+nDfuuXFGHQ/3OIye2KMIawxvuV2hP5ZwA+fETlZLl+13+bxh+u4 RGxTAQVJ7cS5+76Il1tDxsUqXr5UjYEhZmhsiyTBIWGijevWv7LSEG80S9mt/lQiszQJ ZT48i+XtHUmKt2nV1toIttuK5jMvqaaFaXLYWIHiS2HMZfeMLjmw4YvmDrKKpPr8A8uD YvGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723304047; x=1723908847; 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=OTINblrLLv6TSGj30iqQEK49jLsnazYBVdiDP8hf71E=; b=TclsmVyusVHIULNL8LIr+DhFPHyVVLC9C0XH9oPoh2Pnk1SEsXmjl84j9E2qBfTK+E LvGzJpHNwz/0rRVkxnrvza7eW2O5zexo0LPRHJ/fR/dSiIlNbAYim8yHGHrR1y13vA1j 5iUITi436ORibpCKAAiPj9i8oANAakSHi2hEHZTQ8cZIIc6ONcrgkvW7vEQrgEbACpzr /S9p4yei30oMEnYmnH0UfG55bSW9+WawlDlyJfdStTXT9XIArPtOMXFmJC/2fYxq1Wyx Ffy8zlZrzt1gczlSsgwMKUiK9D6D3xaqMQ1/IurdNZ3K+mKVpBTMnImhoauZlLx3NM9f ri0g== X-Gm-Message-State: AOJu0Yz7GAFkzB5OHgOj6r10V1oMY1it4Poo/QiQkmno1BXOUTs0ousC Uyi1AJJshtQWPedaKfu2ht1DBs0eqHt9ivgndex+6dGyfKzZME2JZj/E0g== X-Google-Smtp-Source: AGHT+IGbxiWIIhdPfBBqtqUjiQfYGohHx1AM55RPyFIYbNSl2qLnqJ8upY+W4Z/27nXoHJ6GZ0hAig== X-Received: by 2002:a17:902:cece:b0:1fb:1b16:eb7b with SMTP id d9443c01a7336-200ae4d1120mr66052425ad.16.1723304046573; Sat, 10 Aug 2024 08:34:06 -0700 (PDT) Received: from [192.168.0.14] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-200bbb48b81sm12842015ad.297.2024.08.10.08.34.05 for <ffmpeg-devel@ffmpeg.org> (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 10 Aug 2024 08:34:05 -0700 (PDT) Message-ID: <0bc7bbda-7cc1-428a-9ee8-6749212d6e27@gmail.com> Date: Sat, 10 Aug 2024 12:34:16 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240806221853.959177-1-michael@niedermayer.cc> <20240806221853.959177-5-michael@niedermayer.cc> <79221741-358b-4c9a-9782-51799f2eb416@gmail.com> <20240808212701.GC4991@pb2> <CABPLASQvQ4pTOYSmgiX0QWRPSOj-bqgCztZk+cQJWczRgmTyhw@mail.gmail.com> <20240809200904.GD4991@pb2> Content-Language: en-US From: James Almer <jamrial@gmail.com> In-Reply-To: <20240809200904.GD4991@pb2> Subject: Re: [FFmpeg-devel] [PATCH 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org> List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>, <mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe> List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel> List-Post: <mailto:ffmpeg-devel@ffmpeg.org> List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help> List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>, <mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe> Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org> Archived-At: <https://master.gitmailbox.com/ffmpegdev/0bc7bbda-7cc1-428a-9ee8-6749212d6e27@gmail.com/> List-Archive: <https://master.gitmailbox.com/ffmpegdev/> List-Post: <mailto:ffmpegdev@gitmailbox.com> On 8/9/2024 5:09 PM, Michael Niedermayer wrote: > Hi > > On Fri, Aug 09, 2024 at 03:56:42AM +0200, Kacper Michajlow wrote: >> On Fri, 9 Aug 2024 at 00:06, Michael Niedermayer <michael@niedermayer.cc> wrote: >>> >>> On Thu, Aug 08, 2024 at 02:13:12PM -0300, James Almer wrote: > [...] >>> If decoders are fed with uninitialized buffers thats a >>> security issue because there are thousands if not ten thousands of >>> pathes if you consider the number of decoders and the number >>> of ways they can hit errors >> >> Clearing those buffers in fuzzers does not alleviate this security >> issue, as they may still be uninitialized in production code. > > The decoders in production clear the buffers. The fuzzer does not > so the issues it shows dont exist in production > > look yourself in get_buffer.c > > pool->pools[i] = av_buffer_pool_init(size[i] + 16 + STRIDE_ALIGN - 1, > CONFIG_MEMORY_POISONING ? > NULL : > av_buffer_allocz); > its av_buffer_allocz I disagree. That's from avcodec_default_get_buffer2(). What about DR1 decoders where the caller is using their own avctx.get_buffer2() callback? Nothing in the documentation says that the buffers must be zeroed. I wrote the function you just changed with the intention of finding issues a library user could trigger, which included allocating buffers exactly as big as needed (with no extra padding) and not zeroing it, using lavu helpers like the get_buffer2() documentation states. This change here makes half of that moot, and is hiding potential bugs in the form of use of uninitialized memory in our decoders. > > > [...] >>> Security wise this is not possible for production code, its too >>> fragile (at least with the number of decoders and active maintainers we have) >>> (you want less code to have to be bugfree for security not more code having >>> to be bug free) >>> >>> Now this is the fuzzer and not production code, ok. And of course is >>> great to have error concealment in every decoder >>> But then this leaves the question, who will do this work? >>> If noone does it then we will accumulate many msan bugs in ossfuzz that we wont >>> be able to do much with except ignore them. >>> This would make the fuzzer less efficient and it would confuse people looking >>> at the issues >> >> MSAN is not forgiving, and I can imagine that stabilizing it could >> take time. > >> However, suppressing the reports will not make it more >> efficient. > > It will make it more efficient because then the fuzzer shows only issues > also affecting production and ones someone intends to work on > Otherwise it shows many issues that will distract and confuse > > >> I might not fully understand what you meant, though. > > Yes, i think we misunderstand each other a bit > > > [...] > >> Perhaps it >> should be configurable per decoder. > > That is what i suggested, or at least i meant to. > For decoders where someone intends to fix every case where original buffer > data with nothing written into it come through it could make sense to enable > uninitialized input buffers. > Still i have not seen anyone actually want to do that. I certainly dont have the > time for any of the decoders that i maintain. But if someone else wants > i surely dont mind if (s)he turns this on and works on the additional cases for > any decoders that i maintain ... > > >> >>> Or the short punchy reply maybe is >>> Produce a volunteer who will fix these bugs before declaring them bugs. >>> And when doing so consider that we have bugfixes on the mailing list for which we >>> seem to not even have the man power to review and apply them >>> >>> so yeah my oppinion is the default should be the simple & easy to maintain way. >>> If someone declares their decoder to have flawless error concealment (and for some >>> simple decoders that could be quite simple) these can always be excluded and use >>> uninitialized buffers in the fuzzer >> >> What is the problem with keeping those reports and letting "someone" >> work on their decoder based on reports? > > ossfuzz is the problem, > these issues are not seperate/segregated nor do i see a way ossfuzz could > seperate them but again ATM we have noone intending to work on this so > this patch solves it. > > 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".