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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- 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".