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

[Michael Niedermayer]

On Wed, May 31, 2017 at 01:13:50PM +0200, wm4 wrote:
> On Wed, 31 May 2017 12:51:35 +0200
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, May 31, 2017 at 11:52:06AM +0200, wm4 wrote:
> > > On Wed, 31 May 2017 11:29:56 +0200
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > >   
> > > > On Wed, May 31, 2017 at 09:03:34AM +0200, Hendrik Leppkes wrote:  
> > > > > On Wed, May 31, 2017 at 2:09 AM, Michael Niedermayer
> > > > > <michael at niedermayer.cc> wrote:    
> > > > > > On Wed, May 31, 2017 at 01:14:58AM +0200, Hendrik Leppkes wrote:    
> > > > > >> On Wed, May 31, 2017 at 12:52 AM, Michael Niedermayer
> > > > > >> <michael at niedermayer.cc> wrote:    
> > > > > >> > This prevents an exploit leading to an information leak
> > > > > >> >
> > > > > >> > The existing exploit depends on a specific decoder as well.
> > > > > >> > It does appear though that the exploit should be possible with any decoder.
> > > > > >> > The problem is that as long as sensitive information gets into the decoder,
> > > > > >> > the output of the decoder becomes sensitive as well.
> > > > > >> > The only obvious solution is to prevent access to sensitive information. Or to
> > > > > >> > disable hls or possibly some of its feature. More complex solutions like
> > > > > >> > checking the path to limit access to only subdirectories of the hls path may
> > > > > >> > work as an alternative. But such solutions are fragile and tricky to implement
> > > > > >> > portably and would not stop every possible attack nor would they work with all
> > > > > >> > valid hls files.
> > > > > >> >
> > > > > >> > Found-by: Emil Lerner and Pavel Cheremushkin
> > > > > >> > Reported-by: Thierry Foucu <tfoucu at google.com>
> > > > > >> >
> > > > > >> >    
> > > > > >>    
> > > > > >    
> > > > > >> I don't particularly like this. Being able to dump a HLS stream (ie.
> > > > > >> all its file) onto disk and simply open it again is a good thing.
> > > > > >> Maybe it should just be smarter and only allow using the same protocol
> > > > > >> for the segments then it already used for the m3u8 file, so that a
> > > > > >> local m3u8 allows opening a local file (plus http(s), in case I only
> > > > > >> saved the playlist), but a http HLS playlist only allows http
> > > > > >> segments?    
> > > > > >
> > > > > > we already prevent every protocol except file and crypto for local
> > > > > > hls files. We also already block http* in local hls files by default
> > > > > > thorugh default whitelists (file,crypto for local files)
> > > > > >
> > > > > > This is not sufficient, the exploit there is successfully puts the
> > > > > > content of a readable file choosen by the attacker into the output
> > > > > > video, which if its given back to the attacker leaks this information.
> > > > > >    
> > > > >     
> > > >   
> > > > > Well, I want to be able to store a HLS playlist and its segments
> > > > > locally and play it, without specifying some obscure flag. So how can
> > > > > we make that work?    
> > > > 
> > > > What you ask for is to use vulnerable code (hls with local files is
> > > > pretty much vulnerable by design).
> > > > Enabling this by default is a bad idea and it would be also an
> > > > exception to how its handled in other demuxers.
> > > > For example mov has drefs disabled without the enable_drefs flag.
> > > > 
> > > > Other means than a flag could be used to let the user enable it.
> > > > Would this be what you had in mind ? If so what did you had in mind
> > > > exactly ?
> > > > 
> > > >   
> > > > > 
> > > > > In general, information disclosure is always a risk when you process
> > > > > unvalidated HLS streams, even if you load a remote http playlist which
> > > > > only contains HTTP links, it could reference something on your
> > > > > intranet and get access to something otherwise unavailable to the
> > > > > attacker.    
> > > >   
> > > > > I would put the responsibility of ensuring this doesn't happen on
> > > > > people creating transcoding services, not making our protocols barely
> > > > > usable.    
> > > > 
> > > > According to the authors of the exploit, which they kindly seem to
> > > > have sent to every affected company but not to us.
> > > > Most if not all the big names had hls enabled and were vulnerable.
> > > > So "its the users responsibility" alone did clearly not lead to secure
> > > > code.
> > > > 
> > > > Also users cannot review the ever growing feature set we have for
> > > > security sensitive features and disable them, thats not practical nor
> > > > would it be reasonable from us to ask them to do that.
> > > > 
> > > > If you see a better solution than to disable hls or local file use in
> > > > hls by default then please explain what you suggest ?  
> > > 
> > > How is this "vulnerable"? If it's via the completely useless and
> > > annoying bullshit tty decoder, I'm going to throw a party.  
> > 
> > As stated in the commit message of the patch
> > "The existing exploit depends on a specific decoder as well.
> >  It does appear though that the exploit should be possible with any decoder."
> > 
> > The exploit uses a decoder which we can disable for hls but it would
> > not do anything to stop the vulnerability.
> > We do not have a tty decoder. We have a tty demuxer, the tty demuxer is
> > not used in the exploit.
> 
> Well, the tty demuxer / ansi decoder. The combination allows you to
> dump any text file as video. This would assume the attacker gets the
> video back somehow (transcoder service?).
> 
> > Also from the commit message:
> > "The problem is that as long as sensitive information gets into the decoder,
> >  the output of the decoder becomes sensitive as well."
> > 
> > This should be quite obvious. If you feed some headers + sensitive
> > data into a decoder, the output can be used to reconstruct the
> > sensitive data
> 

> The commit message doesn't really explain how that is a security issue.

hls is a playlist with links to files to play
if hls can refer to local files then the output of transcoding a
mallicous hls file can contain any file readable to the user.
That can be his passwords, private keys, and so on

This renders the output of the transcoder unusable for many uses and
the user is unaware of the potential sensitive content.


> 
> > Its easier with some decoders, harder with others but it will work
> > with almost all decoders with some effort. The existing exploit, not
> > suprisingly uses  a decoder with which the effort to reconstruct the
> > data is minimal.
> > 
> > [...]
> 
> I'm not sure why you keep talking about decoders. Decoders just
> transform data (unless they have bugs, which should be fixed). Isn't
> the problem that the "leaked" data from local filesystem access is
> going somewhere it shouldn't. (Again, I can only imagine a transcoder
> service that accepts anything without being sandboxed. That's pretty
> stupid in itself.)

Theres no question that automated transcoding services should be using
a sandbox, that of course is very true.
Still this problem is not limited to automated transcoding services.

A user downloading a mallicous file (which does not need to end in .hls)
and then transcoding it can put private data of the user into the
output file. That being unknown to the user. if she shares this file
she also shares the data embeded in the file.
This is both a security and privacy issue.


> 
> As as Hendrik already said, you can get the same with http access. An
> attacker could poke around in your local network, which could provide
> access to normally firewalled and private services. Or does your
> attack scenario randomly stop here.
> 
> Also, this doesn't prevent that a HLS file on disk accesses network,
> which could be another problem with services that consume user files
> without further security measures. An attacker could for example upload
> a media file (that really is a m3u8 file) to make the server connect to
> an attacker's server.

The default whitelist for local hls does not contain any network
protocols. So unless the user application or user overrides this or
theres a bug unknown to us there is no network access from local hls


> 
> I'm also not very welcoming of the added message. How is a user of a
> software using FFmpeg supposed to know how to set a libavformat option?

by reading the manual of the software she is using.


> 
> The real fix is to leave sanitation of external accesses to the API
> user by overriding the AVFormatContext.io_open callback.

There are problems with this
1. There is no io_open callback in all affected and supported releases.

2. the io_open callback where it is supported did not stop the exploit.
   in practice.

3. How would an application like ffmpeg and ffplay actually do the
   sanitation ?
   We need to allow access if the user enabled mov dref for example.
   As well as any other such cases.
   also we need to allow accesses to any manually specified pathes and
   files. images with wildcard expansion, ...
   This would result in a rather messy and complex callback
   with many special cases. Security fixes should be as simple as
   possible.

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.

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/20170531/9b473843/attachment.sig>