[FFmpeg-devel] [PATCH] avformat/hls: Check file extensions

Michael Niedermayer michael at niedermayer.cc
Sat Jun 3 15:58:19 EEST 2017


On Sat, Jun 03, 2017 at 11:18:46AM +0200, Hendrik Leppkes wrote:
> On Sat, Jun 3, 2017 at 2:31 AM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> > On Fri, Jun 02, 2017 at 09:27:16PM +0200, Hendrik Leppkes wrote:
> >> On Fri, Jun 2, 2017 at 9:19 PM, Michael Niedermayer
> >> <michael at niedermayer.cc> wrote:
> >> > This reduces the attack surface of local file-system and local network
> >> > information leaking.
> >> >
> >> > It prevents the existing exploit leading to an information leak. As
> >> > well as similar hypothetical attacks.
> >> >
> >> > Leaks of information from files and symlinks ending in common multimedia extensions
> >> > are still possible. But files with sensitive information like private keys and passwords
> >> > generally do not use common multimedia filename extensions.
> >> >
> >> > 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.
> >> >
> >> > Developers have expressed their dislike / objected to disabling hls by default as well
> >> > as disabling hls with local files. This here is a less robust but also lower
> >> > inconvenience solution.
> >> > It can be applied stand alone or together with other solutions.
> >> >
> >>
> >> One of the most important things is that at the very least HLS keeps
> >> working out of the box without magic options for actual HTTP HLS
> >> streams, ie. their primary domain.
> >> One aspect of this is that HLS streams hosted by CDNs often don't have
> >> an actual extension, since they get generated by various dynamic URLs,
> >> and this would break that, so its not a good idea.
> >
> > please provide an example that fails with the patch
> >
> >
> 
> I couldn't find a public HLS stream right away that uses no
> extensions, but I do know that they exist and its easy to craft one
> just by renaming the segments and editing the playlist - I had issues
> with this in an  Android app I was working on last year.

Do you object to fixing this security issue that has a working exploit?
Can you provide a testcase the fix breaks ? So i can look into what
can be done about it ? (multiple testcases would be even better so
theres a lower chance of breaking things)


> 
> Here is another one that fails with this patch:
> 
> Query Parameters after the filename:
> http://daserste_live-lh.akamaihd.net/i/daserste_de@91204/master.m3u8

ill post a patch that fixes this


> 
> Another side-effect, it seems to get stuck and never finish opening
> this file with the blacklist in place (well, at least not for a couple
> minutes, "never" is more time then I have).

Tried it, here it immedeatly stops and exits with
[hls,applehttp @ 0x7f7388000980] Format not on whitelist '-hls,ALL'
http://daserste_live-lh.akamaihd.net/i/daserste_de@91204/master.m3u8: Invalid argument


> 
> I also found some other cases of different extensions being used, ie.
> some streaming servers for example seem to use "m4s" for fragmented
> MP4 in HLS.

extension will be in the next revission of the patch


> The key point here is, extensions are rather meaningless, we don't use
> them for probing files if we can avoid  it, but we do use them now to
> exclude files?

We would use them to limit what kind of information can be accessed and
leaked by default.

Every fix for this vulnerability will cause some (at least hypothetical)
case to break. one can put videos in a subdirectory called .ssh with
the names  of the standard files in there. And any fix that protects
your private ssh key will break that case.
Some fixes disable more by default some disable less.

Ive written 3 fixes for this, and in each thread i asked what people
prefer and asked for people to submit an alternative fix (iam doing
so here too

Do you prefer to disable hls with local files as in the first patch?
Do you prefer to disable hls via whitelist as in the 2nd patch?
Do you prefer to check filenames to be multimedia files as in this
patch ?
Do you want to submit an alternative fix ?

Thanks

[...]
-- 
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/20170603/061663f2/attachment.sig>


More information about the ffmpeg-devel mailing list