[FFmpeg-devel] [PATCH] ffmdec: sanitize codec parameters

[Michael Niedermayer]

On Sat, Nov 19, 2016 at 05:30:59PM +0100, Andreas Cadhalpun wrote:
> On 19.11.2016 16:28, Michael Niedermayer wrote:
> > On Sat, Nov 19, 2016 at 02:04:44PM +0100, Andreas Cadhalpun wrote:
> >> On 19.11.2016 02:14, Michael Niedermayer wrote:
> >>> On Fri, Nov 18, 2016 at 10:35:29PM +0100, Andreas Cadhalpun wrote:
> >>>> On 18.11.2016 02:40, Michael Niedermayer wrote:
> >>>>> On Thu, Nov 17, 2016 at 07:35:01PM +0100, Andreas Cadhalpun wrote:
> >>>>>> +                if (size < 0 || size >= FF_MAX_EXTRADATA_SIZE) {
> >>>>>> +                    av_log(s, AV_LOG_WARNING, "Invalid extradata size %d\n", size);
> >>>>>
> >>>>> i think this and possibly others should be a hard failure
> >>>>> or why would a invalid extradata_size be ok ?
> >>>>
> >>>> The decoder might still be able to work.
> >>>> What would be the advantage of a hard failure?
> >>>
> >>> the advantage of stoping clearly invalid data early
> >>> The decoder runs on a seperate machine with ffm possibly. The file
> >>> just gets demuxed and sent over the network. The warning hinting to
> >>> the issue is on the sending side. The decoder failure at the receiver
> >>> i didnt try but if iam not mixing things up that would be confusing
> >>> neither side would fully understand whats going on without the other
> >>
> >> OK, I changed the extradata_size case to an error.
> >> Which others do you think should be changed, too?
> > 
> > why do you want to accept any invalid values ?
> > ffm files should only be generated by FFmpeg, why should FFmpeg
> > generate invalid files or what is the use case where this occurs ?
> > It feels like iam missing something here ...
> 
> A hard failure is unnecessary and Carl Eugen [1] and Hendrik [2]
> convinced me that it should be avoided.

I think there are 2 different things here
unavailable data, like a bitrate of 0
and
wrong data like a negative bitrate

We do not accept wrong values in general, why should it be different
here ?

ffm files are AFAIK generally temporary (on disk fifo) files generated
by the current muxer.
Is our muxer buggy ?
If it is not where would invalid values come from ?



[...]
-- 
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/20161119/b682ed1b/attachment.sig>