[FFmpeg-devel] [PATCH 2/2] lavu/tests/opts: add tests for filepath options

[J. Dekker]

On 5 Mar 2022, at 20:16, Michael Niedermayer wrote:

> On Fri, Mar 04, 2022 at 04:03:07PM +0100, Niklas Haas wrote:
>> From: Niklas Haas <git at haasn.dev>
>>
>> Using the venerable HEADER.txt as a small file to load.
>> ---
>>  libavutil/tests/opt.c    | 38 +++++++++++++++++++++++++++++++++++++-
>>  tests/fate/libavutil.mak |  2 +-
>>  tests/ref/fate/opt       |  4 ++++
>>  3 files changed, 42 insertions(+), 2 deletions(-)
>
> Please add tests which tries to load
> id_rsa
> ~/.ssh/id_rsa
> shadow
> /etc/shadow
> .bash_history
> ...
>
> The idea here is of course that such attempts fail

There is absolutely no way we can or should try to implement a path based blacklist. Untrusted inputs should be sanitised externally by whichever script is being used to call ffmpeg.

> Also document the security implications of this feature in
> doc/APIchanges / release notes if there is a security implication
>
> Adjusting the parameters of most components could previously
> not read arbitrary files so a application could previously
> pass a string from a untrusted user to it.
> If this changes it needs to be justfied and documented
> If it doesnt change and its still safe that should be documented.
> If it depends on whitelists and callbacks that should be actually implemented
> in ffmpeg and the relevant examples
>
> And i do like this feature, if it can be done without security issues

There aren't any extra security implications here, if a user is allowed to specify filter arguments themselves then they can already use the movie/amovie filter etc. This new option is just a way to unify the way in which filters which already (and will) require to load files can do so.

-- 
J. Dekker