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

[Michael Niedermayer]

On Sun, Nov 20, 2016 at 12:37:05AM +0100, Andreas Cadhalpun wrote:
> On 19.11.2016 22:36, Michael Niedermayer wrote:
> > On Sat, Nov 19, 2016 at 05:30:59PM +0100, Andreas Cadhalpun wrote:
> >> 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,
> 
> If you say so. I'm more interested in fixing the crash than continuing
> this discussion, so I changed it to always error out. See attached patch.
> 
> > ffm files are AFAIK generally temporary (on disk fifo) files generated
> > by the current muxer.
> 
> If that's the case, why does the demuxer still support FFM1 even though
> the muxer can't create such files anymore?

The muxer has created FFM1 files previously,
The muxer should not have created files with invalid values previously
though.
I dont know if no user would be affected if we drop FFM1 support

But with invalid values this is different, they should not have
been created and if they were created our muxer is or was buggy
we need to know about this so we can fix the muxer and stop more
invalid files from being created.
If we make our demuxer accept all invalid fields we would not find
out (no bug reports) about muxer bugs and would then not be able to
fix these

If there is a muxer which did create files with invalid values then
we should support these in the demuxer after fixing the muxer IMO


> 
> Best regards,
> Andreas
> 

>  ffmdec.c |   52 ++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 10 deletions(-)
> a565a670592ad6455e9f26e95edf9886b426dfb3  0001-ffmdec-validate-codec-parameters.patch
> From d3f33b108fe7035e1aa1bc7021dbb99aecd14821 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Thu, 17 Nov 2016 00:04:57 +0100
> Subject: [PATCH] ffmdec: validate codec parameters
> 
> A negative extradata size for example gets passed to memcpy in
> avcodec_parameters_from_context causing a segmentation fault.

LGTM

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: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161120/18d3071c/attachment.sig>