[FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

TODO: bump minor version, add docs

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/avformat.h      | 10 ++++++++++
 libavformat/options.c       | 29 +++++++++++++++++++++++++++++
 libavformat/options_table.h |  3 +++
 3 files changed, 42 insertions(+)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 1916aa2dc5..5ff77323ba 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
      * @return 0 on success, a negative AVERROR code on failure
      */
     int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
+
+    /**
+     * Perform basic same origin checks in default io_open()
+     * - encoding: set by user
+     * - decoding: set by user
+     */
+    int same_origin_check;
+#define AVFMT_SAME_ORIGIN_CHECK_NONE 0  //no check
+#define AVFMT_SAME_ORIGIN_CHECK_HOST 1  //protocol, host, auth, port
+#define AVFMT_SAME_ORIGIN_CHECK_PATH 2  //protocol, host, auth, port, parent path
 } AVFormatContext;
 
 /**
diff --git a/libavformat/options.c b/libavformat/options.c
index e4a3aceed0..7db4bc9b38 100644
--- a/libavformat/options.c
+++ b/libavformat/options.c
@@ -26,6 +26,7 @@
 #include "libavcodec/codec_par.h"
 
 #include "libavutil/avassert.h"
+#include "libavutil/avstring.h"
 #include "libavutil/internal.h"
 #include "libavutil/intmath.h"
 #include "libavutil/opt.h"
@@ -148,6 +149,34 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb,
 
     av_log(s, loglevel, "Opening \'%s\' for %s\n", url, flags & AVIO_FLAG_WRITE ? "writing" : "reading");
 
+    if (s->same_origin_check) {
+        URLComponents uc;
+        int err;
+        size_t len;
+        const char *end;
+        err = ff_url_decompose(&uc, s->url, NULL);
+        if (err < 0)
+            return err;
+
+        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH) {
+            end = uc.query;
+            while (end > uc.path && *end != '/')
+                end--;
+        } else
+            end = uc.path;
+
+        len = end - s->url;
+        if (strncmp(url, s->url, len)) {
+            av_log(s, AV_LOG_ERROR, "Blocking url with differnt origin\n");
+            return AVERROR(EIO);
+        }
+        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH &&
+            av_strnstr(url + len, "/../", uc.query - end)) {
+            av_log(s, AV_LOG_ERROR, "Blocking url tricks\n");
+            return AVERROR(EIO);
+        }
+    }
+
     return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
 }
 
diff --git a/libavformat/options_table.h b/libavformat/options_table.h
index 86d836cfeb..da788164f1 100644
--- a/libavformat/options_table.h
+++ b/libavformat/options_table.h
@@ -106,6 +106,9 @@ static const AVOption avformat_options[] = {
 {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
 {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
 {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },
+{"same_origin", "same origin check", OFFSET(same_origin_check)    , AV_OPT_TYPE_INT  , { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
+{"same_host"  , "same protocol, host, port, auth", 0              , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_HOST }, 0, INT_MAX, D|E, "same_origin"},
+{"same_path"  , "same protocol, host, port, auth, parent path", 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
 {NULL},
 };
 
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[James Almer]

On 5/2/2023 4:36 PM, Michael Niedermayer wrote:
> TODO: bump minor version, add docs
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavformat/avformat.h      | 10 ++++++++++
>   libavformat/options.c       | 29 +++++++++++++++++++++++++++++
>   libavformat/options_table.h |  3 +++
>   3 files changed, 42 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 1916aa2dc5..5ff77323ba 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
>        * @return 0 on success, a negative AVERROR code on failure
>        */
>       int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> +
> +    /**
> +     * Perform basic same origin checks in default io_open()
> +     * - encoding: set by user
> +     * - decoding: set by user
> +     */
> +    int same_origin_check;
> +#define AVFMT_SAME_ORIGIN_CHECK_NONE 0  //no check
> +#define AVFMT_SAME_ORIGIN_CHECK_HOST 1  //protocol, host, auth, port
> +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2  //protocol, host, auth, port, parent path
>   } AVFormatContext;
>   
>   /**
> diff --git a/libavformat/options.c b/libavformat/options.c
> index e4a3aceed0..7db4bc9b38 100644
> --- a/libavformat/options.c
> +++ b/libavformat/options.c
> @@ -26,6 +26,7 @@
>   #include "libavcodec/codec_par.h"
>   
>   #include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
>   #include "libavutil/internal.h"
>   #include "libavutil/intmath.h"
>   #include "libavutil/opt.h"
> @@ -148,6 +149,34 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb,
>   
>       av_log(s, loglevel, "Opening \'%s\' for %s\n", url, flags & AVIO_FLAG_WRITE ? "writing" : "reading");
>   
> +    if (s->same_origin_check) {
> +        URLComponents uc;
> +        int err;
> +        size_t len;
> +        const char *end;
> +        err = ff_url_decompose(&uc, s->url, NULL);
> +        if (err < 0)
> +            return err;
> +
> +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH) {
> +            end = uc.query;
> +            while (end > uc.path && *end != '/')
> +                end--;
> +        } else
> +            end = uc.path;
> +
> +        len = end - s->url;
> +        if (strncmp(url, s->url, len)) {
> +            av_log(s, AV_LOG_ERROR, "Blocking url with differnt origin\n");
> +            return AVERROR(EIO);
> +        }
> +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH &&
> +            av_strnstr(url + len, "/../", uc.query - end)) {
> +            av_log(s, AV_LOG_ERROR, "Blocking url tricks\n");
> +            return AVERROR(EIO);
> +        }
> +    }
> +
>       return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
>   }
>   
> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index 86d836cfeb..da788164f1 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -106,6 +106,9 @@ static const AVOption avformat_options[] = {
>   {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
>   {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
>   {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },
> +{"same_origin", "same origin check", OFFSET(same_origin_check)    , AV_OPT_TYPE_INT  , { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
> +{"same_host"  , "same protocol, host, port, auth", 0              , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_HOST }, 0, INT_MAX, D|E, "same_origin"},
> +{"same_path"  , "same protocol, host, port, auth, parent path", 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},

Missing NONE const? And do we want check_path to be default? It's a 
change in behavior.

>   {NULL},
>   };
>   
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 5026 bytes --]

On Tue, May 02, 2023 at 05:00:29PM -0300, James Almer wrote:
> On 5/2/2023 4:36 PM, Michael Niedermayer wrote:
> > TODO: bump minor version, add docs
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavformat/avformat.h      | 10 ++++++++++
> >   libavformat/options.c       | 29 +++++++++++++++++++++++++++++
> >   libavformat/options_table.h |  3 +++
> >   3 files changed, 42 insertions(+)
> > 
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > index 1916aa2dc5..5ff77323ba 100644
> > --- a/libavformat/avformat.h
> > +++ b/libavformat/avformat.h
> > @@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
> >        * @return 0 on success, a negative AVERROR code on failure
> >        */
> >       int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> > +
> > +    /**
> > +     * Perform basic same origin checks in default io_open()
> > +     * - encoding: set by user
> > +     * - decoding: set by user
> > +     */
> > +    int same_origin_check;
> > +#define AVFMT_SAME_ORIGIN_CHECK_NONE 0  //no check
> > +#define AVFMT_SAME_ORIGIN_CHECK_HOST 1  //protocol, host, auth, port
> > +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2  //protocol, host, auth, port, parent path
> >   } AVFormatContext;
> >   /**
> > diff --git a/libavformat/options.c b/libavformat/options.c
> > index e4a3aceed0..7db4bc9b38 100644
> > --- a/libavformat/options.c
> > +++ b/libavformat/options.c
> > @@ -26,6 +26,7 @@
> >   #include "libavcodec/codec_par.h"
> >   #include "libavutil/avassert.h"
> > +#include "libavutil/avstring.h"
> >   #include "libavutil/internal.h"
> >   #include "libavutil/intmath.h"
> >   #include "libavutil/opt.h"
> > @@ -148,6 +149,34 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb,
> >       av_log(s, loglevel, "Opening \'%s\' for %s\n", url, flags & AVIO_FLAG_WRITE ? "writing" : "reading");
> > +    if (s->same_origin_check) {
> > +        URLComponents uc;
> > +        int err;
> > +        size_t len;
> > +        const char *end;
> > +        err = ff_url_decompose(&uc, s->url, NULL);
> > +        if (err < 0)
> > +            return err;
> > +
> > +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH) {
> > +            end = uc.query;
> > +            while (end > uc.path && *end != '/')
> > +                end--;
> > +        } else
> > +            end = uc.path;
> > +
> > +        len = end - s->url;
> > +        if (strncmp(url, s->url, len)) {
> > +            av_log(s, AV_LOG_ERROR, "Blocking url with differnt origin\n");
> > +            return AVERROR(EIO);
> > +        }
> > +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH &&
> > +            av_strnstr(url + len, "/../", uc.query - end)) {
> > +            av_log(s, AV_LOG_ERROR, "Blocking url tricks\n");
> > +            return AVERROR(EIO);
> > +        }
> > +    }
> > +
> >       return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
> >   }
> > diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> > index 86d836cfeb..da788164f1 100644
> > --- a/libavformat/options_table.h
> > +++ b/libavformat/options_table.h
> > @@ -106,6 +106,9 @@ static const AVOption avformat_options[] = {
> >   {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
> >   {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
> >   {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },
> > +{"same_origin", "same origin check", OFFSET(same_origin_check)    , AV_OPT_TYPE_INT  , { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
> > +{"same_host"  , "same protocol, host, port, auth", 0              , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_HOST }, 0, INT_MAX, D|E, "same_origin"},
> > +{"same_path"  , "same protocol, host, port, auth, parent path", 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
> 
> Missing NONE const? 

added
+{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},


> And do we want check_path to be default? It's a change
> in behavior.

is it usefull if its not enabled by default ?


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

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[James Almer]

On 5/2/2023 5:16 PM, Michael Niedermayer wrote:
> On Tue, May 02, 2023 at 05:00:29PM -0300, James Almer wrote:
>> On 5/2/2023 4:36 PM, Michael Niedermayer wrote:
>>> TODO: bump minor version, add docs
>>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavformat/avformat.h      | 10 ++++++++++
>>>    libavformat/options.c       | 29 +++++++++++++++++++++++++++++
>>>    libavformat/options_table.h |  3 +++
>>>    3 files changed, 42 insertions(+)
>>>
>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index 1916aa2dc5..5ff77323ba 100644
>>> --- a/libavformat/avformat.h
>>> +++ b/libavformat/avformat.h
>>> @@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
>>>         * @return 0 on success, a negative AVERROR code on failure
>>>         */
>>>        int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
>>> +
>>> +    /**
>>> +     * Perform basic same origin checks in default io_open()
>>> +     * - encoding: set by user
>>> +     * - decoding: set by user
>>> +     */
>>> +    int same_origin_check;
>>> +#define AVFMT_SAME_ORIGIN_CHECK_NONE 0  //no check
>>> +#define AVFMT_SAME_ORIGIN_CHECK_HOST 1  //protocol, host, auth, port
>>> +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2  //protocol, host, auth, port, parent path
>>>    } AVFormatContext;
>>>    /**
>>> diff --git a/libavformat/options.c b/libavformat/options.c
>>> index e4a3aceed0..7db4bc9b38 100644
>>> --- a/libavformat/options.c
>>> +++ b/libavformat/options.c
>>> @@ -26,6 +26,7 @@
>>>    #include "libavcodec/codec_par.h"
>>>    #include "libavutil/avassert.h"
>>> +#include "libavutil/avstring.h"
>>>    #include "libavutil/internal.h"
>>>    #include "libavutil/intmath.h"
>>>    #include "libavutil/opt.h"
>>> @@ -148,6 +149,34 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb,
>>>        av_log(s, loglevel, "Opening \'%s\' for %s\n", url, flags & AVIO_FLAG_WRITE ? "writing" : "reading");
>>> +    if (s->same_origin_check) {
>>> +        URLComponents uc;
>>> +        int err;
>>> +        size_t len;
>>> +        const char *end;
>>> +        err = ff_url_decompose(&uc, s->url, NULL);
>>> +        if (err < 0)
>>> +            return err;
>>> +
>>> +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH) {
>>> +            end = uc.query;
>>> +            while (end > uc.path && *end != '/')
>>> +                end--;
>>> +        } else
>>> +            end = uc.path;
>>> +
>>> +        len = end - s->url;
>>> +        if (strncmp(url, s->url, len)) {
>>> +            av_log(s, AV_LOG_ERROR, "Blocking url with differnt origin\n");
>>> +            return AVERROR(EIO);
>>> +        }
>>> +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH &&
>>> +            av_strnstr(url + len, "/../", uc.query - end)) {
>>> +            av_log(s, AV_LOG_ERROR, "Blocking url tricks\n");
>>> +            return AVERROR(EIO);
>>> +        }
>>> +    }
>>> +
>>>        return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
>>>    }
>>> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>>> index 86d836cfeb..da788164f1 100644
>>> --- a/libavformat/options_table.h
>>> +++ b/libavformat/options_table.h
>>> @@ -106,6 +106,9 @@ static const AVOption avformat_options[] = {
>>>    {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
>>>    {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
>>>    {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },
>>> +{"same_origin", "same origin check", OFFSET(same_origin_check)    , AV_OPT_TYPE_INT  , { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
>>> +{"same_host"  , "same protocol, host, port, auth", 0              , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_HOST }, 0, INT_MAX, D|E, "same_origin"},
>>> +{"same_path"  , "same protocol, host, port, auth, parent path", 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
>>
>> Missing NONE const?
> 
> added
> +{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},

"none" sounds more natural.

> 
> 
>> And do we want check_path to be default? It's a change
>> in behavior.
> 
> is it usefull if its not enabled by default ?

It is, since it can be enabled, like the whitelists and blacklists, but 
the question is if it's preferable to have it enabled. If you consider 
it so, then it's good and i wont oppose it.

> 
> 
> [...]
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 6212 bytes --]

On Tue, May 02, 2023 at 05:57:09PM -0300, James Almer wrote:
> On 5/2/2023 5:16 PM, Michael Niedermayer wrote:
> > On Tue, May 02, 2023 at 05:00:29PM -0300, James Almer wrote:
> > > On 5/2/2023 4:36 PM, Michael Niedermayer wrote:
> > > > TODO: bump minor version, add docs
> > > > 
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    libavformat/avformat.h      | 10 ++++++++++
> > > >    libavformat/options.c       | 29 +++++++++++++++++++++++++++++
> > > >    libavformat/options_table.h |  3 +++
> > > >    3 files changed, 42 insertions(+)
> > > > 
> > > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > > > index 1916aa2dc5..5ff77323ba 100644
> > > > --- a/libavformat/avformat.h
> > > > +++ b/libavformat/avformat.h
> > > > @@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
> > > >         * @return 0 on success, a negative AVERROR code on failure
> > > >         */
> > > >        int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> > > > +
> > > > +    /**
> > > > +     * Perform basic same origin checks in default io_open()
> > > > +     * - encoding: set by user
> > > > +     * - decoding: set by user
> > > > +     */
> > > > +    int same_origin_check;
> > > > +#define AVFMT_SAME_ORIGIN_CHECK_NONE 0  //no check
> > > > +#define AVFMT_SAME_ORIGIN_CHECK_HOST 1  //protocol, host, auth, port
> > > > +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2  //protocol, host, auth, port, parent path
> > > >    } AVFormatContext;
> > > >    /**
> > > > diff --git a/libavformat/options.c b/libavformat/options.c
> > > > index e4a3aceed0..7db4bc9b38 100644
> > > > --- a/libavformat/options.c
> > > > +++ b/libavformat/options.c
> > > > @@ -26,6 +26,7 @@
> > > >    #include "libavcodec/codec_par.h"
> > > >    #include "libavutil/avassert.h"
> > > > +#include "libavutil/avstring.h"
> > > >    #include "libavutil/internal.h"
> > > >    #include "libavutil/intmath.h"
> > > >    #include "libavutil/opt.h"
> > > > @@ -148,6 +149,34 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb,
> > > >        av_log(s, loglevel, "Opening \'%s\' for %s\n", url, flags & AVIO_FLAG_WRITE ? "writing" : "reading");
> > > > +    if (s->same_origin_check) {
> > > > +        URLComponents uc;
> > > > +        int err;
> > > > +        size_t len;
> > > > +        const char *end;
> > > > +        err = ff_url_decompose(&uc, s->url, NULL);
> > > > +        if (err < 0)
> > > > +            return err;
> > > > +
> > > > +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH) {
> > > > +            end = uc.query;
> > > > +            while (end > uc.path && *end != '/')
> > > > +                end--;
> > > > +        } else
> > > > +            end = uc.path;
> > > > +
> > > > +        len = end - s->url;
> > > > +        if (strncmp(url, s->url, len)) {
> > > > +            av_log(s, AV_LOG_ERROR, "Blocking url with differnt origin\n");
> > > > +            return AVERROR(EIO);
> > > > +        }
> > > > +        if (s->same_origin_check == AVFMT_SAME_ORIGIN_CHECK_PATH &&
> > > > +            av_strnstr(url + len, "/../", uc.query - end)) {
> > > > +            av_log(s, AV_LOG_ERROR, "Blocking url tricks\n");
> > > > +            return AVERROR(EIO);
> > > > +        }
> > > > +    }
> > > > +
> > > >        return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist);
> > > >    }
> > > > diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> > > > index 86d836cfeb..da788164f1 100644
> > > > --- a/libavformat/options_table.h
> > > > +++ b/libavformat/options_table.h
> > > > @@ -106,6 +106,9 @@ static const AVOption avformat_options[] = {
> > > >    {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D },
> > > >    {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D},
> > > >    {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D },
> > > > +{"same_origin", "same origin check", OFFSET(same_origin_check)    , AV_OPT_TYPE_INT  , { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
> > > > +{"same_host"  , "same protocol, host, port, auth", 0              , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_HOST }, 0, INT_MAX, D|E, "same_origin"},
> > > > +{"same_path"  , "same protocol, host, port, auth, parent path", 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX, D|E, "same_origin"},
> > > 
> > > Missing NONE const?
> > 
> > added
> > +{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},
> 
> "none" sounds more natural.

ill change it


> 
> > 
> > 
> > > And do we want check_path to be default? It's a change
> > > in behavior.
> > 
> > is it usefull if its not enabled by default ?
> 
> It is, since it can be enabled, like the whitelists and blacklists, but the
> question is if it's preferable to have it enabled. If you consider it so,
> then it's good and i wont oppose it.

the problem with default-disabled is that the user needs to know
1. that the option exist
2. what the option does
3. what an attacker can do with such urls
4. that its not enabled by default

OTOH if its enabled by default, the worst it can do is fail with a error
the user can lookup the error and disable the option

but i may be missing something here, also comments both from people
who regularly work with hls and anything else contaning urls in files
and also people who dealt with any related attacks is welcome.

The goal is that this actually does something useful in reality.

thx

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Anton Khirnov]

Quoting Michael Niedermayer (2023-05-02 21:36:31)
> TODO: bump minor version, add docs
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/avformat.h      | 10 ++++++++++
>  libavformat/options.c       | 29 +++++++++++++++++++++++++++++
>  libavformat/options_table.h |  3 +++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 1916aa2dc5..5ff77323ba 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
>       * @return 0 on success, a negative AVERROR code on failure
>       */
>      int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> +
> +    /**
> +     * Perform basic same origin checks in default io_open()
> +     * - encoding: set by user
> +     * - decoding: set by user
> +     */
> +    int same_origin_check;
> +#define AVFMT_SAME_ORIGIN_CHECK_NONE 0  //no check
> +#define AVFMT_SAME_ORIGIN_CHECK_HOST 1  //protocol, host, auth, port
> +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2  //protocol, host, auth, port, parent path

Shouldn't these be flags instead?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Anton Khirnov]

Quoting Michael Niedermayer (2023-05-02 23:15:46)
> the problem with default-disabled is that the user needs to know
> 1. that the option exist
> 2. what the option does
> 3. what an attacker can do with such urls
> 4. that its not enabled by default
> 
> OTOH if its enabled by default, the worst it can do is fail with a error
> the user can lookup the error and disable the option
> 
> but i may be missing something here, also comments both from people
> who regularly work with hls and anything else contaning urls in files
> and also people who dealt with any related attacks is welcome.
> 
> The goal is that this actually does something useful in reality.

This changes behavior in an incompatible way, so IMO this should happen
on a major bump. There should also be a note in the changelog.

Perhaps there could be a special 'auto' value that would initially
default to no effect, but would print a warning if a URL would stop
working after the bump.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Hendrik Leppkes]

On Tue, May 2, 2023 at 10:57 PM James Almer <jamrial@gmail.com> wrote:
> >
> > added
> > +{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},
>
> "none" sounds more natural.
>
> >
> >
> >> And do we want check_path to be default? It's a change
> >> in behavior.
> >
> > is it usefull if its not enabled by default ?
>
> It is, since it can be enabled, like the whitelists and blacklists, but
> the question is if it's preferable to have it enabled. If you consider
> it so, then it's good and i wont oppose it.
>

Is there any estimation how many legitimate streams would be broken by
these options?
If any major streams don't work with this, then its not a good option,
and eg. library users will likely just turn it off or to a lower
setting, as proper streams just have to work - and log output is
pretty much useless for API usage cases.

A quick check for example shows that even something as simple as the
HLS BBC Radio streams will fail _all_ checks, since the playlists are
hosted on another host entirely as the media, thanks to akamai live
streaming.
Playlist here, as an example:
http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2255 bytes --]

On Wed, May 03, 2023 at 12:05:54PM +0200, Hendrik Leppkes wrote:
> On Tue, May 2, 2023 at 10:57 PM James Almer <jamrial@gmail.com> wrote:
> > >
> > > added
> > > +{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},
> >
> > "none" sounds more natural.
> >
> > >
> > >
> > >> And do we want check_path to be default? It's a change
> > >> in behavior.
> > >
> > > is it usefull if its not enabled by default ?
> >
> > It is, since it can be enabled, like the whitelists and blacklists, but
> > the question is if it's preferable to have it enabled. If you consider
> > it so, then it's good and i wont oppose it.
> >
> 
> Is there any estimation how many legitimate streams would be broken by
> these options?
> If any major streams don't work with this, then its not a good option,
> and eg. library users will likely just turn it off or to a lower
> setting, as proper streams just have to work - and log output is
> pretty much useless for API usage cases.
> 
> A quick check for example shows that even something as simple as the
> HLS BBC Radio streams will fail _all_ checks, since the playlists are
> hosted on another host entirely as the media, thanks to akamai live
> streaming.
> Playlist here, as an example:
> http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8

yes, thats why it says RFC in the subject, i had expected that a bit already

still OTOH, blocking these by default is the safer option, i mean if a user
does a
./ffplay http://trustedfoobar.org/cutevideo.avi

would she expect that video to access http://127.0.0.1/ and later http://evilhost/localwebscan-success
I think this should not be possible by default settings, its unexpected

maybe a whitelist of hosts or urls. Something the user could add
*.akamaized.net
to may be an option

Thx

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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Rémi Denis-Courmont]

Nit: different

But is there an actual threat model whence it is necessary or even useful for a media framework to implement origin policies? On top of my head, this can be used by content providers to prevent third parties from referencing their media files... but that seems user-hostile; it does not provide any security for the user of FFmpeg.

I could be wrong, but IMU, origin policy is meant to prevent harmful embedding of images and frames, and to prevent cross-site scripting, but FFmpeg doesn't support either if these anyway, so it's not concerned.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Hendrik Leppkes]

On Wed, May 3, 2023 at 12:49 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Wed, May 03, 2023 at 12:05:54PM +0200, Hendrik Leppkes wrote:
> > On Tue, May 2, 2023 at 10:57 PM James Almer <jamrial@gmail.com> wrote:
> > > >
> > > > added
> > > > +{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},
> > >
> > > "none" sounds more natural.
> > >
> > > >
> > > >
> > > >> And do we want check_path to be default? It's a change
> > > >> in behavior.
> > > >
> > > > is it usefull if its not enabled by default ?
> > >
> > > It is, since it can be enabled, like the whitelists and blacklists, but
> > > the question is if it's preferable to have it enabled. If you consider
> > > it so, then it's good and i wont oppose it.
> > >
> >
> > Is there any estimation how many legitimate streams would be broken by
> > these options?
> > If any major streams don't work with this, then its not a good option,
> > and eg. library users will likely just turn it off or to a lower
> > setting, as proper streams just have to work - and log output is
> > pretty much useless for API usage cases.
> >
> > A quick check for example shows that even something as simple as the
> > HLS BBC Radio streams will fail _all_ checks, since the playlists are
> > hosted on another host entirely as the media, thanks to akamai live
> > streaming.
> > Playlist here, as an example:
> > http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
>
> yes, thats why it says RFC in the subject, i had expected that a bit already
>
> still OTOH, blocking these by default is the safer option, i mean if a user
> does a
> ./ffplay http://trustedfoobar.org/cutevideo.avi
>
> would she expect that video to access http://127.0.0.1/ and later http://evilhost/localwebscan-success
> I think this should not be possible by default settings, its unexpected
>

Coming from the other side -- If the user needs to set the flag for
nearly all streams, then they are not going to check in the future and
just set it, defeating the purpose of them. At which point we might as
well not burden them.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1831 bytes --]

Hi

On Wed, May 03, 2023 at 02:16:03PM +0300, Rémi Denis-Courmont wrote:
> Nit: different

fixed


> 
> But is there an actual threat model whence it is necessary or even useful for a media framework to implement origin policies? On top of my head, this can be used by content providers to prevent third parties from referencing their media files... but that seems user-hostile; it does not provide any security for the user of FFmpeg.
> 
> I could be wrong, but IMU, origin policy is meant to prevent harmful embedding of images and frames, and to prevent cross-site scripting, but FFmpeg doesn't support either if these anyway, so it's not concerned.

This patch was inspired by a report on ffmpeg-security about SSRF
(for which custom io_open() callback or soem sort of sandboxing/VM can be
 used to avoid it)
 The patch here was intended to explore if we can provide something thats
 better tahn currently by default
 
But the same issue with roles flipped occurs for the end user and the user cannot be expected
to setup a custom io_open() callback for his player
The current code can be also used to poke
around the local network of the user. Which is unexpected by the user
for example a avi file could be probed as a m3u8 playlist and then 
poke around on the local net while mixing that with remote urls
from the timing of the remote accesses the remote party should be able
to infer what happened with the local poking. Did it timeout? was
the access rejected ? was there a file that was read and probed/played ?

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Rémi Denis-Courmont]

Le keskiviikkona 3. toukokuuta 2023, 16.33.59 EEST Michael Niedermayer a écrit 
:
> This patch was inspired by a report on ffmpeg-security about SSRF
> (for which custom io_open() callback or soem sort of sandboxing/VM can be
>  used to avoid it)
>  The patch here was intended to explore if we can provide something thats
>  better tahn currently by default

I am not sure how a dodgy HLS manifest would be any different from the user 
clicking an hyperlink from a dodgy website - or opening a dodgy playlist file 
in their FFmpeg-based media player application for that matter. Either way, it 
can open any URL.

It is obviously not an ideal situation, but any restriction here will most 
definitely break existing use cases (and likely be abused by server operators 
to lock FFmpeg out).

Even the "obvious" blocking of secure (HTTPS) to nonsecure (HTTP) references 
is likely to break stuff. If the end result is that everybody just turns origin 
checking off, it's pretty pointless.

> But the same issue with roles flipped occurs for the end user and the user
> cannot be expected to setup a custom io_open() callback for his player
> The current code can be also used to poke
> around the local network of the user. Which is unexpected by the user
> for example a avi file could be probed as a m3u8 playlist and then
> poke around on the local net while mixing that with remote urls
> from the timing of the remote accesses the remote party should be able
> to infer what happened with the local poking.

I agree, but it is unrealistic to change anything here. People make playlists 
mixed with local files and network file systems or cloud storage services. Yes, 
there is a slight information leakage. For instance, you can probe if a local 
file exists by interleaving local and remote URLs in a playlist.

In practice, this is a well-known issue and has been for two at least decades, 
and the "solution" is to limit what the open file can do. To state the obvious 
extreme, one wouldn't want to execute a shell script or an executable from a 
playlist.

-- 
Rémi Denis-Courmont
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3530 bytes --]

On Wed, May 03, 2023 at 07:07:09PM +0300, Rémi Denis-Courmont wrote:
> Le keskiviikkona 3. toukokuuta 2023, 16.33.59 EEST Michael Niedermayer a écrit 
> :
> > This patch was inspired by a report on ffmpeg-security about SSRF
> > (for which custom io_open() callback or soem sort of sandboxing/VM can be
> >  used to avoid it)
> >  The patch here was intended to explore if we can provide something thats
> >  better tahn currently by default
> 
> I am not sure how a dodgy HLS manifest would be any different from the user 
> clicking an hyperlink from a dodgy website - or opening a dodgy playlist file 
> in their FFmpeg-based media player application for that matter. Either way, it 
> can open any URL.

The difference is with a dodgy link its the web browser that has to protect
the user.
With a dodgy HLS file its ffmpeg that has to protect the user.


> 
> It is obviously not an ideal situation, but any restriction here will most 
> definitely break existing use cases (and likely be abused by server operators 
> to lock FFmpeg out).

My goal is to make it more secure by default and to keep it reasonable convenient
to the user

So to me at least, if i open an hls file and i get a popup asking me
"foobar.hls wants to access http://localhost/someexploit,"
"[Allow] [Deny] [Allow Always] [Deny Always]"
thats a win and i wouldnt call that "Breaking" an existing usecase.
Nor is that allowing any server operator to lock FFmpeg out

For a non GUI app thats a matter of adjusting the command line or defaults
by more classical means.

If we can make this more convenient to the user while keeping it secure we should. 
But we should not make it more convenient than what can be done securely.


> 
> Even the "obvious" blocking of secure (HTTPS) to nonsecure (HTTP) references 
> is likely to break stuff. If the end result is that everybody just turns origin 
> checking off, it's pretty pointless.
> 
> > But the same issue with roles flipped occurs for the end user and the user
> > cannot be expected to setup a custom io_open() callback for his player
> > The current code can be also used to poke
> > around the local network of the user. Which is unexpected by the user
> > for example a avi file could be probed as a m3u8 playlist and then
> > poke around on the local net while mixing that with remote urls
> > from the timing of the remote accesses the remote party should be able
> > to infer what happened with the local poking.
> 
> I agree, but it is unrealistic to change anything here. People make playlists 
> mixed with local files and network file systems or cloud storage services. Yes, 
> there is a slight information leakage. For instance, you can probe if a local 
> file exists by interleaving local and remote URLs in a playlist.

I dont know what software you are using but FFmpeg will prevent this attack
with default protocol whitelists

If you have a hls file that mixes local files and remote http(s) then you
need to override the default protocol whitelist.
If iam the user i would do that for that one file which in fact in that case
really has to be my file i wrote. Of course nothing stops the user to set that
by default for all urls, thats the users choice, but i think its much wiser not
to do that


thx

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2854 bytes --]

On Wed, May 03, 2023 at 02:24:34PM +0200, Hendrik Leppkes wrote:
> On Wed, May 3, 2023 at 12:49 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Wed, May 03, 2023 at 12:05:54PM +0200, Hendrik Leppkes wrote:
> > > On Tue, May 2, 2023 at 10:57 PM James Almer <jamrial@gmail.com> wrote:
> > > > >
> > > > > added
> > > > > +{"same_none"  , "same origin check off"                       , 0 , AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E, "same_origin"},
> > > >
> > > > "none" sounds more natural.
> > > >
> > > > >
> > > > >
> > > > >> And do we want check_path to be default? It's a change
> > > > >> in behavior.
> > > > >
> > > > > is it usefull if its not enabled by default ?
> > > >
> > > > It is, since it can be enabled, like the whitelists and blacklists, but
> > > > the question is if it's preferable to have it enabled. If you consider
> > > > it so, then it's good and i wont oppose it.
> > > >
> > >
> > > Is there any estimation how many legitimate streams would be broken by
> > > these options?
> > > If any major streams don't work with this, then its not a good option,
> > > and eg. library users will likely just turn it off or to a lower
> > > setting, as proper streams just have to work - and log output is
> > > pretty much useless for API usage cases.
> > >
> > > A quick check for example shows that even something as simple as the
> > > HLS BBC Radio streams will fail _all_ checks, since the playlists are
> > > hosted on another host entirely as the media, thanks to akamai live
> > > streaming.
> > > Playlist here, as an example:
> > > http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
> >
> > yes, thats why it says RFC in the subject, i had expected that a bit already
> >
> > still OTOH, blocking these by default is the safer option, i mean if a user
> > does a
> > ./ffplay http://trustedfoobar.org/cutevideo.avi
> >
> > would she expect that video to access http://127.0.0.1/ and later http://evilhost/localwebscan-success
> > I think this should not be possible by default settings, its unexpected
> >
> 
> Coming from the other side -- If the user needs to set the flag for
> nearly all streams, then they are not going to check in the future and
> just set it, defeating the purpose of them. At which point we might as
> well not burden them.

Yes, we need a system that is secure and works in most cases.

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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Rémi Denis-Courmont]

Le keskiviikkona 3. toukokuuta 2023, 22.05.26 EEST Michael Niedermayer a écrit 
:
> On Wed, May 03, 2023 at 07:07:09PM +0300, Rémi Denis-Courmont wrote:
> The difference is with a dodgy link its the web browser that has to protect
> the user. With a dodgy HLS file its ffmpeg that has to protect the user.

Well, the browser does not protect the user in that case. At best, an 
antimalware plugin or proxy might catch it but that's pretty much it. In fact, 
origin checks don't protect against this, nor are they intended to (AFAIK).

> > It is obviously not an ideal situation, but any restriction here will most
> > definitely break existing use cases (and likely be abused by server
> > operators to lock FFmpeg out).
> 
> My goal is to make it more secure by default and to keep it reasonable
> convenient to the user
> 
> So to me at least, if i open an hls file and i get a popup asking me
> "foobar.hls wants to access http://localhost/someexploit,"
> "[Allow] [Deny] [Allow Always] [Deny Always]"
> thats a win and i wouldnt call that "Breaking" an existing usecase.
> Nor is that allowing any server operator to lock FFmpeg out

User dialogues are notoriously bad for security purposes, as users don't 
really have the necessary info to make the proper decision, and just learn to 
click Allow Always. All they achieve is absolve the developers from protecting 
the user, really.

But even if, what will the criterion be? You'll match localhost. Okay. What 
about localhost.localdomain and 127.0.0.1? What about all the noncanonical 
writings of 127.0.0.1 and other addresses in 127.0.0.0/8, and their 
noncanonical writing? IPv6? What about the assigned external IP address of the 
host And so on, and that's just to prevent connections to localhost.

Some browsers prevent connections to private IP addresses too, to "protect" 
the Intranet, but the flip side is to break corporate content and home file 
servers.

> If we can make this more convenient to the user while keeping it secure we
> should. But we should not make it more convenient than what can be done
> securely.

I appreciate the thought but without a clearly defined threat model, you can't 
define the security issue, and thus you can't hope to fix it. Meanwhile, you 
will break things, and if you're not careful, you might as well make it easier 
for publishers to deliberately block FFmpeg.

> If you have a hls file that mixes local files and remote http(s) then you
> need to override the default protocol whitelist.

It makes sense for HLS manifests because HLS is by definition tied to HTTP, but 
that won't work for real playlists (M3U or other).

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Timo Rothenpieler]

On 03.05.2023 21:08, Michael Niedermayer wrote:
>>>> A quick check for example shows that even something as simple as the
>>>> HLS BBC Radio streams will fail _all_ checks, since the playlists are
>>>> hosted on another host entirely as the media, thanks to akamai live
>>>> streaming.
>>>> Playlist here, as an example:
>>>> http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
>>>
>>> yes, thats why it says RFC in the subject, i had expected that a bit already
>>>
>>> still OTOH, blocking these by default is the safer option, i mean if a user
>>> does a
>>> ./ffplay http://trustedfoobar.org/cutevideo.avi
>>>
>>> would she expect that video to access http://127.0.0.1/ and later http://evilhost/localwebscan-success
>>> I think this should not be possible by default settings, its unexpected
>>>
>>
>> Coming from the other side -- If the user needs to set the flag for
>> nearly all streams, then they are not going to check in the future and
>> just set it, defeating the purpose of them. At which point we might as
>> well not burden them.
> 
> Yes, we need a system that is secure and works in most cases.

What about doing what actual browsers do, and reading the 
Access-Control-Allow-Origin HTTP header, and checking if the current 
origin is allowed?

This does not really work for local files. Best you could do is check 
for "*" or not.
But would at least fix the BBC+Akamai case.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1889 bytes --]

On Wed, May 03, 2023 at 11:01:43PM +0200, Timo Rothenpieler wrote:
> On 03.05.2023 21:08, Michael Niedermayer wrote:
> > > > > A quick check for example shows that even something as simple as the
> > > > > HLS BBC Radio streams will fail _all_ checks, since the playlists are
> > > > > hosted on another host entirely as the media, thanks to akamai live
> > > > > streaming.
> > > > > Playlist here, as an example:
> > > > > http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
> > > > 
> > > > yes, thats why it says RFC in the subject, i had expected that a bit already
> > > > 
> > > > still OTOH, blocking these by default is the safer option, i mean if a user
> > > > does a
> > > > ./ffplay http://trustedfoobar.org/cutevideo.avi
> > > > 
> > > > would she expect that video to access http://127.0.0.1/ and later http://evilhost/localwebscan-success
> > > > I think this should not be possible by default settings, its unexpected
> > > > 
> > > 
> > > Coming from the other side -- If the user needs to set the flag for
> > > nearly all streams, then they are not going to check in the future and
> > > just set it, defeating the purpose of them. At which point we might as
> > > well not burden them.
> > 
> > Yes, we need a system that is secure and works in most cases.
> 
> What about doing what actual browsers do, and reading the
> Access-Control-Allow-Origin HTTP header, and checking if the current origin
> is allowed?
> 
> This does not really work for local files. Best you could do is check for
> "*" or not.
> But would at least fix the BBC+Akamai case.

I like the idea, do you want to implement it ?

thx

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".