[FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()

[Alexander Strasser]

On 2014-10-26 02:49 +0200, Michael Niedermayer wrote:
> On Sun, Oct 26, 2014 at 02:09:19AM +0200, wm4 wrote:
> > On Sun, 26 Oct 2014 00:16:26 +0200
> > Nicolas George <george at nsup.org> wrote:
> > 
> > > Le quartidi 4 brumaire, an CCXXIII, Michael Niedermayer a écrit :
> > > > This fixes the issue that a not set or not forwarded whitelist
> > > > would allow to bypass it.
> > > 
> > > I am a bit worried by the sheer amount of extra code and API this simple
> > > feature requires. Maybe it is a sign that it was not the best approach to
> > > begin with.
> > > 
> > > Also, I did not say at the time, but I believe using a string to hold a list
> > > like that is quite ugly.
> > > 
> > > Unless I am mistaken, the core of the problem resides in the existence of
> > > global variables in the library, and this is not the only instance of this
> > > issue: remember people who want to free the global mutex manager, not
> > > understanding that "still reachable" is not a memory leak.
> > > 
> > > There is an idea that I have been nursing for some time: moving all these
> > > global variables to a structure, and start passing it around everywhere.
> > > Most of the functions that access the global variables already have some
> > > kind of context where the pointer can be stored, so it would only require
> > > changing a few API entry points.
> > > 
> > > That may look somewhat like that:
> > > 
> > > AVCodecInstance *libavcodec = avcodec_library_create(OPTIONS);
> > > AVFormatInstance *libavformat = avformat_library_create(libavformat, OPTIONS);
> > > 
> > > avformat_instance_open_input(libavformat, &ctx, filename, NULL, NULL);
> > > 
> > > Then the whitelist feature comes for free: use the options to disable
> > > registering all the codecs, then manually register the ones needed.
> > > 
> > > Of course, for compatibility, a global instance need to be inited if the
> > > legacy entry points are used, but it becomes much easier to test extensively
> > > that modern API usage will not touch them.
> > > 
> > > Of course, that requires more work, and the whitelist feature will not be
> > > available immediately, but in the long run I believe it would be greatly
> > > beneficial.
> > > 
> > > What do people think of that?
> > 
> > That's exactly the same idea I suggested some hours ago on IRC - so
> > it's not good, I guess?
> 
> the idea is only good with a volunteer to implement it.
> the funny thing is i had thought about using such context too before
> i read about it today from you and nicolas.
> I didnt further look into it when i thought about it as it seemed
> mostly orthogonal to the whitelists.
> Its a nicer place to put them in but it doesnt really change things.
> Also its a moderate amount of work and looks like it would require
> user apps to be updated to a new API again which isnt all that great

  Basically I agree with things said so far, but not everything is
needed right now. A whitelist feature OTOH is really useful. Especially
when using system FFmpeg, which is usually build with all the code
build-able usable on the platform.

  Besides storing the whitelist in a (long,flat) string internally might
(and probably should) be replaced (at some point), I consider it essential
hat FFmpeg continues to provide this interface on the outside (API) to make
the whitelist feature readily usable by applications. It would be bad if
every application has to invent glue code, to make the feature usable and
persistable etc. It would leave more room for usage errors and a false
sense of security. (That does not exclude providing the feature through
other interfaces too, like what Nicolas suggested.)


  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141026/0d40333f/attachment.asc>