On 31.05.2017 18:33, Michael Niedermayer wrote:
On Wed, May 31, 2017 at 05:18:57PM +0200, Tobias Rapp wrote:
On 31.05.2017 15:42, wm4 wrote:
On Wed, 31 May 2017 14:49:19 +0200
Michael Niedermayer <mich...@niedermayer.cc> wrote:

[...]

Security fixes should be as simple as
  possible.

Well, your fix isn't simple. It adds yet another exception with
questionable effect. It makes it more complex and harder to predict
what will actually happen, not simpler.

If people want, I can limit the local file check to the case where
the io_open callback is not set?
That way user applications which do their own sanitation would not be
affected by the check or error message and stay in full control of
what access is allowed.

That would have little value and would make it more complex too.

I'd say a good way to make this secure would be disabling the hls
protocol in builds which are security sensitive.

We already have "protocol_whitelist", --disable-protocol and
application sandboxing as supported and generic options. I agree
with wm4 that some special case-handling here just adds complexity.

"--disable-protocol" does not allow fixing this, the vulnerability
only needs the file protocol ultimatly.

similarly protocol_whitelist only helps if "file" is not on it,
no other protocol is really required for this.

I just confirmed the exploit works with
-protocol_whitelist file,crypto

sandboxing is the awnser for automated transcoding services but
the average joe end user cannot use sandboxing

What do you suggest ?

Well as far as I understand the user must
(a1) be tricked into opening a playlist file with FFmpeg
or
(a2) some software based on FFmpeg libraries, then
(b) be tricked into uploading the output file to a server under control of the attacker.

Adding another command-line argument will not help much for (a1) as user Joe Average might not find/understand it in the long list of options (or the options are simply hidden behind some script - but in case a script is executed we have lost already anyway).

For (a2) the application should put playlist muxers on the blacklist, if not required for normal usage. When being extra cautious this could be the library option's default.

Now (b) is the biggest part of the security breach. Uploading non-verified binary data to unknown third parties is bad. Thats the reason why Joe Average should also opt-out to all that telemetry or crash report uploading done by other applications.

But if Joe is willing to upload some data which has been generated via some untrusted commands he is not far from uploading other readable local files anyway - I see no meaningful way to prevent that from code within FFmpeg.

Regards,
Tobias

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to