[FFmpeg-devel] [PATCH 2/3] avformat: reject FFmpeg-style merged side data in raw packets

[wm4]

On Wed, 8 Mar 2017 20:15:24 +0100
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Wed, Mar 08, 2017 at 07:35:50PM +0100, wm4 wrote:
> > On Wed, 8 Mar 2017 19:20:15 +0100
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >   
> > > On Wed, Mar 08, 2017 at 05:26:57PM +0100, wm4 wrote:  
> > > > On Wed, 8 Mar 2017 17:11:12 +0100
> > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > >     
> > > > > On Wed, Mar 08, 2017 at 04:06:20PM +0100, wm4 wrote:    
> > > > > > On Wed, 8 Mar 2017 15:36:25 +0100
> > > > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > > > >       
> > > > > > > On Wed, Mar 08, 2017 at 01:40:11PM +0100, wm4 wrote:      
> > > [...]  
> > > > >    
> > > > > >       
> > > > > > > also it may be interresting to disable this check for fuzzing so
> > > > > > > side data can be fuzzed in a wider range of cases and any past
> > > > > > > testcases that happen to use this can still be used for regression
> > > > > > > testing      
> > > > > > 
> > > > > > I think what you want is fault injection for memory errors, seems out
> > > > > > of scope here.      
> > > > > 
> > > > > no, i want fuzzing to continue to fuzz side data, it did so in the
> > > > > past and it should continue to do so.    
> > > > 
> > > > You can fuzz side data as much as you can fuzz AVFrame or
> > > > AVCodecContext. I believe randomly changing in-memory data structures
> > > > is referred to as fault injection, not fuzzing.    
> > > 
> > > it doesnt really matter what you call it, but it was done and the
> > > patch breaks it if theres no option to disable it or something else  
> > 
> > PS: honestly, I think you're trolling with this. The possibility that
> > random packets could be interpreted as side-data doesn't look like
> > something that was accounted for, and I highly doubt this plays a role
> > for fuzzing (or ever occurred in a fuzzing case). Why are you so
> > hell-bent in finally preventing this arguably dangerous corner case? It
> > makes no sense at all. Explain yourself.  
> 
> It plays a role, for example the timeout in 731 goes away if
> i change FF_MERGE_MARKER and the value prior to teh change is in the
> sample according to my hex editor
> 
> so the fuzzer seems capable to create data to get past the check
> iam not trolling and this does play a role, ive seen other issues
> that used the side data split
> 
> how the fuzzer does this i dont know but i can imagine a few ways
> like looking at the individual bytes of the comparission and evolving
> towards breaking it or maybe some input file was generated by
> libavformat leaking the marker into it somehow.

If you argue this way, the fuzzer will expose issues that can happen
with the normal data flow too. (For example, if a demuxer sets a field
in a side data struct to a value read from the file - obviously the
fuzzer will be capable of exposing bugs related in readers of this
field.)

Normally you would be glad that an issue the fuzzer previously exposed
goes away completely.