[FFmpeg-devel] [PATCH 0/3] Finish new iteration APIs

[Rostislav Pehlivanov]

On 20 February 2018 at 16:30, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Tue, Feb 20, 2018 at 10:17:02AM -0300, James Almer wrote:
> > On 2/20/2018 9:21 AM, wm4 wrote:
> > > On Tue, 20 Feb 2018 08:47:51 -0300
> > > James Almer <jamrial at gmail.com> wrote:
> > >
> > >>> It has fully achieved its objectives. There's no more visible global
> > >>> mutable state, and the actual global mutable state has been reduced
> to
> > >>> less than 5 codec entries.
> > >>
> > >> It's true that after the deprecation period they will effectively be
> the
> > >> only ones still mutable, but shouldn't the objective be to cover them
> all?
> > >
> > > That would be nice. But this has been discussed before. No kind of
> > > different registration API could fix this issue either, unless we start
> > > mallocing AVCodec structs and require the user to free them, or
> > > something equally absurd. The proper solution for this specific issue
> > > would be making a new API that somehow replaces AVCodec.pix_fmts.
> > >
> > >>>
> > >>> Why are we discussing this _again_?
> > >>
> > >> Because a drawback has been found, namely that link time optimization
> > >> using static libraries can't remove "non registered" modules anymore,
> > >> and now depends fully on what's enabled at configure time.
> > >> Ideally, a better registration based API that follows what i described
> > >> above should be designed with care.
> > >
> > > Oh yeah, bikeshed attack by Michael. As it was said a few thousand
> times
> > > already, public API users could NOT use the registration stuff to
> > > register only specific components at runtime. So they have to use the
> > > register_all variants, which pull in _all_ components even with static
> > > linking.
> >
> > Oh, i assumed it was possible to use avcodec_register() on a manual
> > basis in a proper API usage scenario, but i see now that's not the case.
>
> of course its possible to use avcodec_register() on a manual basis in a
> proper API usage scenario, why would it not be ?
>
> why do you think this function exists or was written ?
>
> is it the ff_* symbols you are thinking of ?
> This is a problem for an existing API it is not a problem for a new API as
> we can change the symbols if they are intended to be used for individual
> component registration.
> The whole discussion is about designing a new API. So any limitation of
> an existing API can be changed.
>
> There also should be no need to call register_all in the existing API,
> and there cannot be such a problem in a new design because it would be
> a new design you wouldnt design it to "not work".
>
>
> >
> > Nevermind then, my argument was focused on preventing losing some link
> > time optimization for static libraries assuming proper API usage.
> >
> > >
> > > And that can't be made with dynamic linking either. If you statically
> > > link anyway, you probably control everything, and you can configure
> this
> > > stuff at compile time.
> > >
> > > Then I guess there's this very special case where a fuzzer statically
> > > links to libavcodec, and registers only a few components (technically
> > > violating the API and by guessing the component symbol name), and
> > > without calling the register_all functions. This would make the
> > > resulting executable much smaller, which is apparently important
> > > because Google (who runs the fuzzers for their oss-fuzz project) is too
> > > poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> > > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > > wrong code. In addition, it's apparently infeasible to just build
> > > every fuzzer with a separate libavcodec. (Not sure about the details,
> it
> > > was something like this.)
> > >
> > > Those requirements are really quite nebulous. I don't know why we even
> > > should care - surely whoever does this will find a solution that works
> > > for them. For example they could apply a small patch that makes the
> > > codec_list[] symbol non-static non-const, which would allow them to
> > > overwrite it (i.e. deleting unneeded entries). It's a really simple
> > > solution that took me 5 minutes to come up with.
> >
> > Something like this is probably the best solution for the fuzzer issue,
> yes.
>
> This basically says one should fork ffmpeg if one has problems with the
> new API
>
> Thats a very chilling response to be honest in a discussion about that APIs
> design. More so as this is at a time we still can change the API.
>
> and anyone who wants to only register a subset of components (due to
> security)
> has to either link to a seperate binary (which is  disallowed in some
> major distributions which mandate shared libs without exception IIRC so
> that
> sounds great but simple isnt possible)
> or just not use FFmpeg or fork it or use a fork or just forget about only
> registering a subset.
>

You bring up the security argument but I don't see anything here which
would compromise it. _All_ the executable code for all codecs is still in
the library. Restricting the codecs you get during init would do nothing
for security if the attacker can still call any function.