[FFmpeg-devel] [PATCH] avformat/hls: Disallow local file access by default

Michael Niedermayer michael at niedermayer.cc
Thu Jun 1 13:13:35 EEST 2017


On Thu, Jun 01, 2017 at 11:02:09AM +0200, Tobias Rapp wrote:
> 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 <michael at 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

yes, though
a1/a2 are required for any attack involving input data. The user
must open a file/stream provided by an attacker.
She doesnt know its a playlist, in fact the exploit we have is not
identifyable as a playlist, neither by file extension nor by the
file tool.


> (b) be tricked into uploading the output file to a server under
> control of the attacker.

Depending on what goal the attacker has, half of this is not needed

For example if the attacker just wants to harm his victim he does
not need access to the file. Making her spread her private keys or
passwords vissibly in the metadata or other part of videos is all thats
needed

For another attack the attacker just needs to upload the exploit well
covered up at various random places. Random people downloading the
videos then will realize the files are unexpectedly large and unwieldy
for the quality and some will transcode them and some will share them.
Especially when the video is a bit "viral"
The shared videos would contain their private keys.
At this point the attacker could with some luck just google for the
video title and collect the private keys of random people from the
resulting files. (for this the attacker would of course not place
the private keys in the metadata vissibly but put them elsewhere where
they require some elaborate processing to extract so they arent
found by anyone else)
Thats one potential attack with this vulnerability that doesnt
require the more difficult steps of getting a specific individual to
do something. I very much doubt thats the only or worst possible attack


> 
> 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

The patch causes a note to be printed pointing to the option if it is
the cause of a file being rejected.


> (or the options are simply hidden behind some script - but
> in case a script is executed we have lost already anyway).

we are not responsible for security issues in 3rd party scripts
nor are we really able to do anythiing about that.
If a script does something insecure thats the script authors fault
There are also scripts that set things randomly to world writable
to fix permission issues ...

But we are responsible for security in our software, that is FFmpeg.


> 
> 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.

There is a protocol blacklist but no demuxer blacklist.
even if we add such list, it would be missing in all releases


> 
> 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.

i fully agree, but theres no way joe average can verify a output file
to be clean before upload. Nor does joe (even if he is quite smart)
expect the video he transcoded to contain some bits of private local
files.

This vulnerability is someting that we need to fix.
Do you have an idea thats better than what the patch in this thread
does ?

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170601/ae522a56/attachment.sig>


More information about the ffmpeg-devel mailing list