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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3328 bytes --]

On Thu, Aug 08, 2024 at 02:13:12PM -0300, James Almer wrote:
> On 8/6/2024 7:18 PM, Michael Niedermayer wrote:
> > Fixes: use of uninitialized values
> > Fixes: 70885/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP6F_fuzzer-4610946029387776 (and likely others)
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   tools/target_dec_fuzzer.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
> > index d2d7e21dac7..794b5b92cc7 100644
> > --- a/tools/target_dec_fuzzer.c
> > +++ b/tools/target_dec_fuzzer.c
> > @@ -129,7 +129,7 @@ static int fuzz_video_get_buffer(AVCodecContext *ctx, AVFrame *frame)
> >       frame->extended_data = frame->data;
> >       for (i = 0; i < 4 && size[i]; i++) {
> > -        frame->buf[i] = av_buffer_alloc(size[i]);
> > +        frame->buf[i] = av_buffer_allocz(size[i]);
> >           if (!frame->buf[i])
> >               goto fail;
> >           frame->data[i] = frame->buf[i]->data;
> 
> Wouldn't this hide actual decoder bugs too?

iam not sure i understand what you mean

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
Pathes in which these buffers are not filled completely, so each
of these pathes would then need to clear the right bits of data.
Basically that means implementing error concealment for every decoder.
AND making sure that error concealment code is 100% bugfree and leaves
never a spot uncleaned and never touched something that was not writen to
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

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

thx

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".