[FFmpeg-devel] [PATCH 1/2] avcodec: Add AV_CODEC_FLAG2_FAST_UNSAFE, move unsafe uses of FAST to it

[Michael Niedermayer]

On Thu, May 28, 2020 at 01:43:17PM -0300, James Almer wrote:
> On 5/28/2020 1:20 PM, Michael Niedermayer wrote:
> > TODO: Bump
> > 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  doc/APIchanges             | 3 +++
> >  doc/codecs.texi            | 2 ++
> >  libavcodec/avcodec.h       | 6 ++++++
> >  libavcodec/h264dec.c       | 2 +-
> >  libavcodec/options_table.h | 1 +
> >  tools/target_dec_fuzzer.c  | 2 +-
> >  6 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index fb5534b5f5..3e20a44379 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil:     2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2020-xx-xx - xxxxxxxxxx - lavc 58.xx.100 - avcodec.h
> > +  Add AV_CODEC_FLAG2_FAST_UNSAFE
> > +
> >  2020-xx-xx - xxxxxxxxxx - lavc 58.88.100 - avcodec.h codec.h
> >    Move AVCodec-related public API to new header codec.h.
> >  
> > diff --git a/doc/codecs.texi b/doc/codecs.texi
> > index c092aadc0e..46790b66b3 100644
> > --- a/doc/codecs.texi
> > +++ b/doc/codecs.texi
> > @@ -787,6 +787,8 @@ Possible values:
> >  @table @samp
> >  @item fast
> >  Allow non spec compliant speedup tricks.
> > + at item fast_unsafe
> > +Allow speedup tricks which can lead to out of array reads and crashes on damaged or crafted files.
> 
> This will raise more than a couple eyebrows. Having an option to enable
> what people will consider security issues is not a good idea at all. For
> starters, it acknowledges lavc is not secure and has known issues that
> are purposely not being fixed.

now, thats not what this was intended to do, of course.
the idea is more

A user can have a stream that is known to be valid, quite possibly
the users own stream, or otherwise "in house" made or already checked
to be valid.

In that case any code that is only needed for invalid streams becomes
unneeded.


> And on top of it, this can't be outright
> disabled/removed at compile time, so something could still call
> ffmpeg/lavc with it enabled.

well, yes, but thats not the only such case
we have other options to enable unsafe behavior


> 
> The issues should be fixed, or the relevant "fast" codepath in the
> decoder removed for being buggy.

h264 is a specific use of this flag, and that might not be the only
place it could be used in

But about h264 What this is about if i remember it correctly, is
that the maximum input any crafted bitstream of a block can require is X,
now you can if the input size is less than X copy that to a larger buffer or
you can add lots of checks. Both of these slow the code down a bit.
OTOH, if the stream is known to be valid that can be skipped.

It can also be skiped if the buffer is already big enough to begin with
OR if the output goes to the parser and not the decoder.
So even without the user having access to this, the codepath does not
become unneeded
the h264 case is more a "even if you cant proof its safe on case 123
use it anyway"
And quite possibly we can add more code detecting more cases where
it is safe, this should be investigated either way probably.

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If the United States is serious about tackling the national security threats 
related to an insecure 5G network, it needs to rethink the extent to which it
values corporate profits and government espionage over security.-Bruce Schneier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200528/92e003db/attachment.sig>