[FFmpeg-devel] [PATCH] avcodec/rl2: set dimensions

[Michael Niedermayer]

On Wed, Jul 24, 2019 at 02:42:24PM +0200, Lynne wrote:
> Jul 24, 2019, 11:08 AM by michael at niedermayer.cc:
> 
> >
> > What did you expect ? IIRC you have asked for whole classes of security
> > issues to be not fixed.
> >
> > Something like that would require a vote and majority of developers.
> >
> 
> The way I interpret this: "Of course I ignored you, you're mental!", doesn't help. And I don't think its just me.

You are reading something into this that i have never meant or written


> And you do remember incorrectly in saying that I want this whole class of security issues not fixed. In this thread I specifically raised the issue of what is considered to be a security issue by asking whether a speedup of failing to decode from 2 to 0.4 seconds is considered such or what's considered acceptable in general.
> And I think I'll disagree that it is. 16 seconds to 2 seconds I can accept, but not 2 to 0.4.

These durations are the testcases found by the fuzzer, they say nothing about
what the worst case for an issue is.
The fuzzer builds a testcase trying to exceed a timeout it stops trying to
"improve" it once it found something that takes a few seconds.

You can in general make these cases significantly longer running.

The reason why the fuzzer doesnt produce hour or day long timeouts is just because
it doesnt search for anything longer than a few seconds.


> 
> 
> 
> >> These patches affect decoding of real world broken files in favor of fixing specially crafted fuzzed files.
> >>
> >
> > Iam happy to look into such cases. Can you provide me with such
> > "real world broken files"?
> > Its not intended to worsen the output from such files
> >
> 
> Simple logical analysis, "if file is somewhat broken, don't try decoding" does very well indicate that it won't only apply for _this_ broken file, but in general.
> Thus, this is for you to prove. I've said it before that otherwise its a burden to other developers to have to screen all of these patches.

The changes i do in general, i think about potential effects on
slightly broken files and try to test with what iam able to find as matching
input material.
I find it a bit rude from you that you assume i would not already consider this
case.
Do i never make a mistake ? well i wish so but iam a human. So again if you
know of specific cases where theres a problem, tell me about them please


> 
> 
> 
> >> Sure, protecting against ddos attacts is important, but not important enough to make decoders give up early and return nothing. Especially in cases where the timeout speedup is of the order of 2s to 400ms.
> >> Yet in all of those timeout patches all you've cared about is shutting up the tool. You've never once shown any figures if this could affect decoding, because its a lot harder than just showing they fix something some tool calls a timeout and forgetting about it. You haven't even commented on this when I asked you on IRC.
> >> You also sneak this type of patches in when there's an overflow later on during decoding, which is completely incorrect in almost all cases, for the same reason above.
> >>
> >
> > if you know of issues in a patch or commit you should report this
> > during patch review or as soon as you find out about the issue
> > as a reply to that patch or commit or as mail to the author.
> >
> 
> That's what I'm doing.
> That aside, you've completely ignored my statements on what's considered acceptable, showing figures, and sneaking this type of patches to fix undefined behavior.
> Making your reply a simple refutation, rather than addressing anything I've said.
> So I'm asking you again, what is considered a security issue and what is considered acceptable? And what is considered not a security issue but a complaint from an overzealous automated tool.

undefined behavior is unacceptable. Its not allowed in C. It doesnt matter
here if its a security issue or not.

Timeouts can in general be used for denial of service attacks. While this
is less critical than many other security issues it is a security issue.
Also for Timeouts many point to bugs, to missing end of input checks, to missing
checks in or before loops, to missing EOF checks, to missing checks that
the input actually contains enough data resembling a fraction of the smallest
half valid frame.

We can spend many hours and days arguing if a issue is critical enough to be
a security issue. maybe the one we would look at is not but then i still would
try to fix it.
If we dont fix one it will block the fuzzer from finding another timeout
issue in the same decoder. And that one could be security relevant.
So fixing as many as possible is the awnser here too

About the fuzzer, if it reports a timeout then there is a timeout,
if it reports undefined behavior then there is undefined behavior.
Always ? no, it contained bugs but that is rather uncommon.

Also the fuzzer has no mercy with you, you dont fix some issue, it will be
made public and security researchers will look into how to exploit it
eventually. If you didnt have the time or manpower, or deadlocked
yourself with arguments the fuzzer doesnt care it has a clock and when thats
up it publishes all details of an issue.

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190725/6e2289c8/attachment.sig>