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

[Michael Niedermayer]

On Thu, Jun 01, 2017 at 12:13:35PM +0200, Michael Niedermayer wrote:
> 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.
[...]
> >
> > 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

ive fixed some bugs and inconsistencies in the whitelists and then
implemented something very similar. Patch sent for that.

That might work with releases up to 2.5



[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/35fe8379/attachment.sig>