[FFmpeg-devel] [PATCH 3/4] avformat/concat: Add concat_enable option that is disable by default

[Paul B Mahol]

On 1/20/16, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Wed, Jan 20, 2016 at 05:06:37PM +0100, Nicolas George wrote:
>> Le primidi 1er pluviose, an CCXXIV, Michael Niedermayer a ecrit :
>> > From: Michael Niedermayer <michael at niedermayer.cc>
>> >
>> > This should prevent the unintended use of concat
>>
>> I am rather against this patch and the corresponding for subfile: these
>> protocols are not harmful by themselves, they are dangerous if and only
>> another protocol or format allows untrusted sources to provide arbitrary
>> URLs.
>
> This is true, but its easy for a user application to pass an
> arbitrary string as URL
> And our documentation does not say that doing so would be dangerous
> In fact it doesnt say much at all
> "@param filename Name of the stream to open."
> is all there is in avformat_open_input() docs
>
> and elsewhere it says
>  * @section lavf_decoding_open Opening a media file
>  * The minimum information required to open a file is its URL or filename,
> which
>  * is passed to avformat_open_input(), as in the following code:
>  * @code
>  * const char    *url = "in.mp3";
>
> That suggests that passing a filename as is, is safe
>
>
>> This kind of preemptive blacklisting is bound to fail (new protocols
>> are added frequently, and they may be more dangerous than just concat or
>> subfile) and only mitigates a few of the possible attacks.
>
> yes
>
> my concern is about the past releases here, and existing 3rd party
> applications which where written based on the docs.
> Its much easier to require these protocols to be explicitly enabled
> than to potentially patch a significant number of applications.
> I fully agree that applications should never pass arbitrary strings
> as URLs, files should start with "file:" and so on
>
>
>
>>
>> If people start to care about playlist-based security issues (Reimar used
>> to
>> warn about it long ago), a cross-protocol solution needs to be found.
>
> thats true for git-master, and i can look into implementing whitelists
> similar to the format&codec whitelists we have but ATM my concern is
> more about the past releases
>
> do you object to this patch being applied to the past releases ?
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
>

The approach is pointless. Either remove concat protocol or dont add
option that disables it.