[FFmpeg-devel] [PATCH 3/3] avformat: set the default whitelist to disable hls

[Michael Niedermayer]

On Mon, Jun 05, 2017 at 05:33:29PM +0200, Nicolas George wrote:
> Le septidi 17 prairial, an CCXXV, Michael Niedermayer a écrit :
[...]
> > You dont need to convince me that the extension check or changes
> > within just hls are not a complete solution. Iam quite well aware
> > of this. This is intended to stop an existing exploit and variants of
> > it in practice and do so quickly.
> 
> It depends on the severity of the threat. This one seems quite minor and
> far-fetched, and thus I think we could take our time to fix it properly.
> We all have noticed that temporary quick-and-dirty fixes usually stay
> here a long time unless whoever implemented them is actively working on
> a real fix.

I disagree that the issue is minor and far fetched.

The exploit that i have was successfully used against multiple
companies (it was a demonstration and AFAIK no harm was done).
That same attack works against all recent releases.

My oppinion was and is that a exploit working basically on 100% of
targets and can leak private information is a serious issue.

I think that if argumentation continues to center around the
lack of importance of this issue then we should try to bring some
outside security experts into this discussion.


[...]
> Still, after having spent a few hours in a train without proper network
> access, and with someone actually interested in listening, I could give
> it some thought.
> 
> I will start by explaining why protocol_whitelist is bad.
> 
> First, it is a string. Enough with strings used as data structures! That
> is inefficient, inconvenient and a maintenance nightmare since extending
> or fixing the syntax will likely be considered an API break by some.
> 
> Second, the issue never was about protocols. ~/tmp/playlist.m3u8 should
> not be allowed to access ../.firefox/cookies.txt (Firefox protects us
> from that by adding a salt in the path, but not all sensitive files are
> protected that way, this is only an illustrative example), and that
> applies to local files, to SMB shares or to SFTP clients;

> http://www.example.com/playlist.m3u8 should not be allowed to access
> http://localhost:631/, yet they are both HTTP.

yes but its worse than this
http://www.example.com:631/playlist.m3u8
and
http://www.example.com:631

can already screw you, if you are not carefull. The reason is the DNS
lookups. these 2 uses can lookup to different IPs, one could be on
the LAN the other the attackers server
If you want to prevent this, I think you need to implement a global
DNS cache. Technically that can be done and i belive browsers are
doing this and doing this for the reason above. iam no browser expert
though it just all fits together ...


> 
> The issue is about subsets of the URL space. Files from one URL should
> be allowed to access data from URLs in the same relevant subset (same
> subdirectory or same web server maybe?), but not outside.

What percentage of hls files out in the wild does this work with ?


> 
> Of course, the protocol is part of the URL. But not all protocols are
> like that: file://, http://, ssh:// are part of the URL, while concat:,
> crypto:, subfile: are not and should be transparent for the mechanism. I
> will call the formers "URL protocols" and the laters "utility
> protocols".
> 
> So let us get rid of that protocol_whitelist and replace it with an
> actual data structure, designed to encode a subset of the URL space. Let
> us call it AVSecurityClearance, at least for the rest of this mail.
> 
> Now, there is something done right with protocol_whitelist: it is, as it
> should be, a property of muxers, demuxers and protocols (and anything
> else that would be allowed to access external data) and must be
> inherited when a component opens another component (demuxer -> protocol,
> or demuxer -> slave demuxer, or protocol -> slave protocol, etc.).
> AVSecurityClearance should work the same way.
> 
> Now, how should we implement that data structure? I do not know!
> 

> Maybe we can restrict it to subsets that are a sub-hierarchy: "anything
> below http://www.example.com/music/".

a file /home/jane/fromweb.hls
should not be allowed to access /home/jane/.ssh/
which would be a sudirectory


> 
> But I suspect AVSecurityClearance will need to be polymorphic: the
> structure of data is not necessarily the same for all protocols.
> 
> If AVSecurityClearance is polymorphic, then implementing union and
> intersection operators is quite easy.
> 
> A few considerations about hierarchy. Some protocols have a host part.
> The syntax of DNS domains is hierarchical from right to left, unlike the
> syntax for file paths. It must be handled separately. The port part
> needs a special treatment too: which ones are closer to
> http://example.com/: http://www.example.com/ or
> http://example.com:8080/? Or maybe they are to be considered all
> separate.
> 
> Also, URLs with numerical IP addresses require a special treatment; we
> do not want to implement a whois client, so we will have to treat them
> as completely separate.
> 
> Another design question: when a master opens a slave and the
> AVSecurityClearance is inherited, should it be copied, like
> protocol_whitelist, or shared? If it is shared, later restrictions
> caused by data access in one component are immediately propagated to all
> the components. I suspect it is the right choice. But then,
> AVSecurityClearance must be refcounted.
> 
> This is all I have for now. And I will not have time to work on this
> anytime soon. But somebody needs to.
> 

> Of course, whoever works on it can come up with their own ideas. But I

> suspect any working solution will work in the ways I just outlined.

yes this sounds plausible.

Though if you dont have time to work on this, i doubt i will work on it
alone anytime soon. I dont want to work on this just to have someone
reject it because it either breaks too many hls files or because of
a global DNS cache if that turns out to be needed ...
Said differently iam happy to help but there need to be more people
than me behind this pushing this forward ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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/20170606/003d59f7/attachment.sig>