From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id EB64F4607C for ; Tue, 2 May 2023 21:15:56 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 685A868B984; Wed, 3 May 2023 00:15:54 +0300 (EEST) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 49BCB689E1F for ; Wed, 3 May 2023 00:15:48 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 9C7D860003 for ; Tue, 2 May 2023 21:15:47 +0000 (UTC) Date: Tue, 2 May 2023 23:15:46 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20230502211546.GB1391451@pb2> References: <20230502193631.10844-1-michael@niedermayer.cc> <20230502201627.GA1391451@pb2> MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============9190412058787005818==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============9190412058787005818== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bCsyhTFzCvuiizWE" Content-Disposition: inline --bCsyhTFzCvuiizWE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 > > > >=20 > > > > Signed-off-by: Michael Niedermayer > > > > --- > > > > libavformat/avformat.h | 10 ++++++++++ > > > > libavformat/options.c | 29 +++++++++++++++++++++++++++++ > > > > libavformat/options_table.h | 3 +++ > > > > 3 files changed, 42 insertions(+) > > > >=20 > > > > 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, po= rt > > > > +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2 //protocol, host, auth, po= rt, 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 =3D ff_url_decompose(&uc, s->url, NULL); > > > > + if (err < 0) > > > > + return err; > > > > + > > > > + if (s->same_origin_check =3D=3D AVFMT_SAME_ORIGIN_CHECK_PA= TH) { > > > > + end =3D uc.query; > > > > + while (end > uc.path && *end !=3D '/') > > > > + end--; > > > > + } else > > > > + end =3D uc.path; > > > > + > > > > + len =3D end - s->url; > > > > + if (strncmp(url, s->url, len)) { > > > > + av_log(s, AV_LOG_ERROR, "Blocking url with differnt or= igin\n"); > > > > + return AVERROR(EIO); > > > > + } > > > > + if (s->same_origin_check =3D=3D AVFMT_SAME_ORIGIN_CHECK_PA= TH && > > > > + 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_cal= lback, options, s->protocol_whitelist, s->protocol_blacklist); > > > > } > > > > diff --git a/libavformat/options_table.h b/libavformat/options_tabl= e.h > > > > index 86d836cfeb..da788164f1 100644 > > > > --- a/libavformat/options_table.h > > > > +++ b/libavformat/options_table.h > > > > @@ -106,6 +106,9 @@ static const AVOption avformat_options[] =3D { > > > > {"max_streams", "maximum number of streams", OFFSET(max_streams)= , AV_OPT_TYPE_INT, { .i64 =3D 1000 }, 0, INT_MAX, D }, > > > > {"skip_estimate_duration_from_pts", "skip duration calculation i= n estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_O= PT_TYPE_BOOL, {.i64 =3D 0}, 0, 1, D}, > > > > {"max_probe_packets", "Maximum number of packets to probe a code= c", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 =3D 2500 }, 0, INT_M= AX, D }, > > > > +{"same_origin", "same origin check", OFFSET(same_origin_check) = , AV_OPT_TYPE_INT , { .i64 =3D 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 =3D 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 =3D AVFMT_SAME_ORIGIN_CHECK_PATH }, 0, INT_MAX,= D|E, "same_origin"}, > > >=20 > > > Missing NONE const? > >=20 > > added > > +{"same_none" , "same origin check off" , 0 , AV= _OPT_TYPE_CONST, { .i64 =3D AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX, D|E= , "same_origin"}, >=20 > "none" sounds more natural. ill change it >=20 > >=20 > >=20 > > > And do we want check_path to be default? It's a change > > > in behavior. > >=20 > > is it usefull if its not enabled by default ? >=20 > It is, since it can be enabled, like the whitelists and blacklists, but t= he > 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 [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus --bCsyhTFzCvuiizWE Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZFF9eQAKCRBhHseHBAsP q+z5AKCcBFqYgqhmx+gRKftmnqeHkaPDxQCdFyKtkXAWoLEvJwqKRKstj/OxhZ8= =MSZX -----END PGP SIGNATURE----- --bCsyhTFzCvuiizWE-- --===============9190412058787005818== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============9190412058787005818==--