[FFmpeg-devel] [PATCH 2/2] avformat: add protocol_whitelist

[Nicolas George]

Le quintidi 5 pluviôse, an CCXXIV, Michael Niedermayer a écrit :
> the argument is added, not out of strict need, (there already is a
> AVDictionary that can be used) but to make it clear that the author
> set the whitelist correctly. Also it simplifies the code compared
> to using the AVDictionary.
> 
> security relevant code should be clear and explicit so that a
> mistake in the code can easily be spotted. Its very hard to spot that
> a passed dictionary doesnt carry/lost the whitelist, easy to spot that
> a argument is missed and set to NULL

I see the point, but since this whitelist is not a full fix, I believe this
is premature.

> we use string whitelists for codecs and formats, using a differnet
> system for protocols would be inconsistent at least

True, but that only means that codecs and format should be fixed as well.

> The struct suggested has a few flaws as well
> URLProtocols are not public so lists of pointers to them in a "AV*"
> that is public struct might cause problems

Not a problem if the structure is completely opaque.

> also if the same is done for codecs and formats you have 3 such
> structs with the need for 3 sets of allocators, deallocators and
> probably a few helper functions for each to construct them.

Actually, I was more thinking of the opposite: a single structure for
codecs, formats, protocols, etc.

> then new function prototypes could be needed for
> encoder, decoder, audio, video, subtitles, muxers, demuxers and
> protocols to pass in these structs. (depending on details of the API)
> also to maintain support for the existng codec/format whitelists
> convertion and merging code would be needed
> 
> this is alot of complexity

Indeed, and a lot of it comes from insufficient planning in the previous
rounds, so let us not do that again.

> another problem of the struct is that depending on from which lib
> the protocols are set the same protocol may have unequal pointers
> 
> which system do people prefer ?
> do we have a volunteer to implement a struct based system ?
> 
> do people want the string based solution to be applied till then
> or to not have this security feature until then ?

Do we want a good fix, or do we want a quick fix? As I explained earlier, a
good fix requires designing a real security policy, not just a stupid
whitelist. It will take time.

In my opinion, the security issue in question is so benign that we can just
take the time to fix it properly. If people do not agree, then a quick fix
is necessary, but then it should not make it harder to implement the correct
fix later. So for starters, either do not introduce new APIs or make it very
clear they are temporary.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160124/399fb207/attachment.sig>