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

[Michael Niedermayer]

On Sun, Dec 11, 2016 at 07:27:26PM +0100, Nicolas George wrote:
> Le primidi 21 frimaire, an CCXXV, Michael Niedermayer a écrit :
> > Well, we need these options for efficient fuzzing
> 
> We need SOMETHING, maybe, but not specifically THESE.
> 
> > There are multiple independant things these options solve
> > I belive i explained all already, but
> 
> You were vague. We only start having details now.
> 

> > Fuzzers search and find issues like out of array accesses but also
> > hangs and oom conditions.
> > Fuzzers find hangs and OOM conditions by executing code until it runs
> > out of memory or reaches a timeout.
> > These cases are then reported and need to be checked by a human (that
> > being me in practice it seems)
> > ATM almost all of reported issues are false positives, going through
> > them takes significant amounts of time. the max_pixels parameter should
> > fix this as all the cases i looked at where hitting out of memory or
> > timeout because of very high resolutions.
> 
> Then run the fuzzers with a low address space limit. Problem solved.

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


[...]

> 
> > The 2nd issue this fixes is a CVE that was reported about a crafted
> > file with i belive 26k streams hitting OOM.
> > If you belive this CVE is invalid and not a security issue iam totally
> > the wrong to discuss that with. But i like to fix issues instead of
> > arguing about why they are or are not a security issue.
> 
> Open a ticket, attach the file, we can all discuss what the proper fix
> is.

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.


[...]

> > > Third step: we discuss and implement a proper solution.
> > I already did that and the previously applied solution is the result.
> 
> It is not a PROPER solution if it brings us into a maintenance
> nightmare.

who is "us", who is affected by this ?
I thought i would be maintaining this alone. Is there someone who
will help and work on this ?

I certainly dont mind the patches being reverted and replaced by
another implementation.
What i do mind is if its reverted and either nothing gets implemented
or if what is implemented doesnt solve the problems this does.


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20161211/a6ba4e77/attachment.sig>