[FFmpeg-devel] [PATCH 1/3] Revert "avcodec: Add max_pixels options"

[Michael Niedermayer]

On Mon, Dec 12, 2016 at 12:04:05PM +0100, Nicolas George wrote:
> Le primidi 21 frimaire, an CCXXV, Michael Niedermayer a écrit :
> > You misunderstand
> > 
> > I want to find code that allocates too much memory where it should
> > not.
> > to give an example
> > there was long ago some code like
> > 
> > len = read()
> > for (i<len)
> >     x= alloc()
> >     x.whatever =read()
> >     ...
> > 
> > Straight OOM here with a tiny input file.
> > add a simple if(eof) in there and no OOM anymore
> > this is just one example, code can look very diferent.
> > 
> > I want to find these cases and i want to fix them
> > But what i get from the fuzzer is files with resolutions like
> > 65123x3210 which go OOM because of valid but silly resolution.
> > If i can limit the resolution then i can find the other issues
> > which i can fix.
> > If i cannot limit the resolution then i cannot fix the other issues
> > as they are in a sea of OOMs from large resolution files
> > 
> > Nothing you can do at the OS level will get you this effect
> 
> Thanks for explaining.
> 
> If I read this correctly, this option does not fix any security issue at
> all, it only help you find other parts of the code that may contain
> security issues. Am I right?

its more complex
there are really independant things this achievs

OOM is a security issue under some(1) circumstances, one can hit OOM due
to too many pixels (or streams), this specific issue is fixed by the
options

completely independant of this, the option is needed to fuzz FFmpeg
efficiently as andreas explained

and also completely independant of this, the option is needed to
allow finding some fixable OOM bugs that i would like to fix


(1) For example a server process that dies due to it. Even if restarted
this can put load on the machine to be a effective was to DOS it

Also its certainly true that this does not fix every OOM issue but
no bugfix that fixes a out of array read fixes every out of array read
either


> 
> > it is exceptionally unprofessional to publish testcases for CVEs
> > before they have been fixed.
> > Also more generally its the researchers choice/job to publish their
> > work. If you belive it should be put in a ticket you should ask him
> > not a 3rd party like me to do that.
> 
> This is Free software, secrecy is not a good policy. "I have this patch
> that fix a bug, but I can not show you the bug." Well, if the patch is
> straightforward, we can accept it, but if the patch is not
> straightforward, we need, collectively, to see the bug.
> 
> I can understand that if the bug is a critical 0-day exploit, some
> leeway must be accepted. But "there is a file that triggers a crash" is
> not enough by far.

If it is neccessary to publish exploits then i belive all security
support will end in FFmpeg and move elsewhere
I cannot really speak for others but i belive that companies doing
fuzzing and submit testcases will not submit them if that implies
them being made public. Actually they can probably not even legally
do that if they wanted it would massivly endanger their customers.

About this specific bug, as mentioned it has a CVE, it is on the public
oss security list
heres a link:
http://www.openwall.com/lists/oss-security/2016/12/08/1

That seems more than adequate to understand this one reported case
I can privatly share the sample with FFmpeg developers who work on
writing an alterantive fix

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161212/ee8022c7/attachment.sig>