[FFmpeg-devel] [PATCH 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content

[Michael Niedermayer]

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 at 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


[...]
> > 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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240809/732d3e8e/attachment.sig>