| Message ID | 20170603192004.1437-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 189ff4219644532bdfa7bab28dfedaee4d6d4021 |
| Headers | show |
On Sat, Jun 03, 2017 at 09:20:04PM +0200, Michael Niedermayer wrote: > This reduces the attack surface of local file-system > 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. > It does not stop leaks via remote addresses in the LAN. > > 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. There also where objections against restricting > remote url file extensions. This here is a less robust but also lower > inconvenience solution. > It can be applied stand alone or together with other solutions. > > Found-by: Emil Lerner and Pavel Cheremushkin > Reported-by: Thierry Foucu <tfoucu@google.com> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/hls.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) Applied with the author name joke suggested by nicolas Thanks [...]
Le septidi 17 prairial, an CCXXV, Michael Niedermayer a écrit :
> Applied with the author name joke suggested by nicolas
Despite Hendrik's objection?
The joke name was nit an approval, and I had no authority to give it
anyway.
(On a phone and bad network, will reply to the other mail later.)
On 6/5/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Jun 03, 2017 at 09:20:04PM +0200, Michael Niedermayer wrote: >> This reduces the attack surface of local file-system >> 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. >> It does not stop leaks via remote addresses in the LAN. >> >> 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. There also where objections against >> restricting >> remote url file extensions. This here is a less robust but also lower >> inconvenience solution. >> It can be applied stand alone or together with other solutions. >> >> Found-by: Emil Lerner and Pavel Cheremushkin >> Reported-by: Thierry Foucu <tfoucu@google.com> >> >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavformat/hls.c | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) > > Applied with the author name joke suggested by nicolas This is joke, please revert this.
On Mon, Jun 05, 2017 at 08:26:34AM +0200, Nicolas George wrote: > Le septidi 17 prairial, an CCXXV, Michael Niedermayer a écrit : > > Applied with the author name joke suggested by nicolas > > Despite Hendrik's objection? I have of course talked with hendrik before pushing. It was him who suggested to limit the check to local files as done in the last posted and applied patch [...]
On Mon, Jun 05, 2017 at 11:13:06AM +0200, Paul B Mahol wrote: > On 6/5/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Sat, Jun 03, 2017 at 09:20:04PM +0200, Michael Niedermayer wrote: > >> This reduces the attack surface of local file-system > >> 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. > >> It does not stop leaks via remote addresses in the LAN. > >> > >> 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. There also where objections against > >> restricting > >> remote url file extensions. This here is a less robust but also lower > >> inconvenience solution. > >> It can be applied stand alone or together with other solutions. > >> > >> Found-by: Emil Lerner and Pavel Cheremushkin > >> Reported-by: Thierry Foucu <tfoucu@google.com> > >> > >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >> --- > >> libavformat/hls.c | 18 +++++++++++++++++- > >> 1 file changed, 17 insertions(+), 1 deletion(-) > > > > Applied with the author name joke suggested by nicolas > > This is joke, please revert this. ok, done [...]
diff --git a/libavformat/hls.c b/libavformat/hls.c index 4b8fb19a52..01731bd36b 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -204,6 +204,7 @@ typedef struct HLSContext { char *http_proxy; ///< holds the address of the HTTP proxy server AVDictionary *avio_opts; int strict_std_compliance; + char *allowed_extensions; } HLSContext; static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) @@ -618,8 +619,19 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url, return AVERROR_INVALIDDATA; // only http(s) & file are allowed - if (!av_strstart(proto_name, "http", NULL) && !av_strstart(proto_name, "file", NULL)) + if (av_strstart(proto_name, "file", NULL)) { + if (strcmp(c->allowed_extensions, "ALL") && !av_match_ext(url, c->allowed_extensions)) { + av_log(s, AV_LOG_ERROR, + "Filename extension of \'%s\' is not a common multimedia extension, blocked for security reasons.\n" + "If you wish to override this adjust allowed_extensions, you can set it to \'ALL\' to allow all\n", + url); + return AVERROR_INVALIDDATA; + } + } else if (av_strstart(proto_name, "http", NULL)) { + ; + } else return AVERROR_INVALIDDATA; + if (!strncmp(proto_name, url, strlen(proto_name)) && url[strlen(proto_name)] == ':') ; else if (av_strstart(url, "crypto", NULL) && !strncmp(proto_name, url + 7, strlen(proto_name)) && url[7 + strlen(proto_name)] == ':') @@ -2134,6 +2146,10 @@ static int hls_probe(AVProbeData *p) static const AVOption hls_options[] = { {"live_start_index", "segment index to start live streams at (negative values are from the end)", OFFSET(live_start_index), AV_OPT_TYPE_INT, {.i64 = -3}, INT_MIN, INT_MAX, FLAGS}, + {"allowed_extensions", "List of file extensions that hls is allowed to access", + OFFSET(allowed_extensions), AV_OPT_TYPE_STRING, + {.str = "3gp,aac,avi,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,wav"}, + INT_MIN, INT_MAX, FLAGS}, {NULL} };
This reduces the attack surface of local file-system 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. It does not stop leaks via remote addresses in the LAN. 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. There also where objections against restricting remote url file extensions. This here is a less robust but also lower inconvenience solution. It can be applied stand alone or together with other solutions. Found-by: Emil Lerner and Pavel Cheremushkin Reported-by: Thierry Foucu <tfoucu@google.com> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/hls.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)