[FFmpeg-devel,2/2,RFC] avformat/avio: Fail on opening non file urls which exist as local files without whitelists

Submitted by Michael Niedermayer on Dec. 5, 2016, 12:52 p.m.

Details

Message ID 20161205125251.28683-2-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Dec. 5, 2016, 12:52 p.m.
TODO: this needs to cleanly open a file url context for checking

This stops someone having a local file like "http:evilhost.com" and playing it
as "http:evilhost.com" without explicitly specifying the http protocol on the whitelist

That is it reduces the impact of people not using the "file:" scheme explicitly on untrusted
filenames at the expense of causing some problems if a remote url exists ad a local file

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/avio.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

wm4 Dec. 5, 2016, 1:15 p.m.
On Mon,  5 Dec 2016 13:52:51 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> TODO: this needs to cleanly open a file url context for checking
> 
> This stops someone having a local file like "http:evilhost.com" and playing it
> as "http:evilhost.com" without explicitly specifying the http protocol on the whitelist
> 
> That is it reduces the impact of people not using the "file:" scheme explicitly on untrusted
> filenames at the expense of causing some problems if a remote url exists ad a local file
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/avio.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index 3606eb0fda..5a11add415 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -31,6 +31,8 @@
>  #endif
>  #include "url.h"
>  
> +static const struct URLProtocol *url_find_protocol(const char *filename);
> +
>  /** @name Logging context. */
>  /*@{*/
>  static const char *urlcontext_to_name(void *ptr)
> @@ -188,6 +190,17 @@ int ffurl_connect(URLContext *uc, AVDictionary **options)
>          return AVERROR(EINVAL);
>      }
>  
> +    if ((uc->flags & AVIO_FLAG_READ) &&
> +        !uc->protocol_whitelist &&
> +        !uc->protocol_blacklist &&
> +        strcmp(uc->prot->name, "file")) {
> +        const URLProtocol *file_protocol = url_find_protocol("file:");
> +        if (file_protocol->url_check(uc, 0) >= 0) {
> +            av_log(uc, AV_LOG_ERROR, "Ambigous filename %s exists, specify a whitelist!\n", uc->filename);

This error message is completely dongs. A whitelist, what?? I don't
care if it makes sense in context of some obscure feature that no user
will know.

This also shouldn't depend on whether the file exists (url_check seems
to do that).

> +            return AVERROR(EEXIST);
> +        }
> +    }
> +
>      if (!uc->protocol_whitelist && uc->prot->default_whitelist) {
>          av_log(uc, AV_LOG_DEBUG, "Setting default whitelist '%s'\n", uc->prot->default_whitelist);
>          uc->protocol_whitelist = av_strdup(uc->prot->default_whitelist);

Patch hide | download patch | download mbox

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 3606eb0fda..5a11add415 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -31,6 +31,8 @@ 
 #endif
 #include "url.h"
 
+static const struct URLProtocol *url_find_protocol(const char *filename);
+
 /** @name Logging context. */
 /*@{*/
 static const char *urlcontext_to_name(void *ptr)
@@ -188,6 +190,17 @@  int ffurl_connect(URLContext *uc, AVDictionary **options)
         return AVERROR(EINVAL);
     }
 
+    if ((uc->flags & AVIO_FLAG_READ) &&
+        !uc->protocol_whitelist &&
+        !uc->protocol_blacklist &&
+        strcmp(uc->prot->name, "file")) {
+        const URLProtocol *file_protocol = url_find_protocol("file:");
+        if (file_protocol->url_check(uc, 0) >= 0) {
+            av_log(uc, AV_LOG_ERROR, "Ambigous filename %s exists, specify a whitelist!\n", uc->filename);
+            return AVERROR(EEXIST);
+        }
+    }
+
     if (!uc->protocol_whitelist && uc->prot->default_whitelist) {
         av_log(uc, AV_LOG_DEBUG, "Setting default whitelist '%s'\n", uc->prot->default_whitelist);
         uc->protocol_whitelist = av_strdup(uc->prot->default_whitelist);