[FFmpeg-devel] [PATCH 1/2] Revert "avformat/mpegts: Add standard extension so hls can check in extension_picky mode"

[FFmpeg-devel] [PATCH 1/2] Revert "avformat/mpegts: Add standard extension so hls can check in extension_picky mode"

[Michael Niedermayer]

The next commit implements the hls fix in a way that doesnt need this

This reverts commit 54897da7ce8ae6e349cd56d0f11cb2404e264efa.
---
 libavformat/mpegts.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 1337aa12030..765bedec5cc 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -3459,7 +3459,6 @@ void avpriv_mpegts_parse_close(MpegTSContext *ts)
 const FFInputFormat ff_mpegts_demuxer = {
     .p.name         = "mpegts",
     .p.long_name    = NULL_IF_CONFIG_SMALL("MPEG-TS (MPEG-2 Transport Stream)"),
-    .p.extensions   = "ts,m4s",
     .p.flags        = AVFMT_SHOW_IDS | AVFMT_TS_DISCONT,
     .p.priv_class   = &mpegts_class,
     .priv_data_size = sizeof(MpegTSContext),
-- 
2.48.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".

[FFmpeg-devel] [PATCH 2/2] avformat/hls: Be more picky on extensions

[Michael Niedermayer]

This blocks disallowed extensions from probing
It also requires all available segments to have matching extensions to the format
mpegts is treated independent of the extension

It is recommended to set the whitelists correctly
instead of depending on extensions, but this should help a bit,
and this is easier to backport

Fixes: CVE-2023-6602 II. HLS Force TTY Demuxer
Fixes: CVE-2023-6602 IV. HLS XBIN Demuxer DoS Amplification

The other parts of CVE-2023-6602 have been fixed by prior commits

Found-by: Harvey Phillips of Amazon Element55 (element55)
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 doc/demuxers.texi |  7 +++++++
 libavformat/hls.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index cc81c615dd7..2324b3b4690 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -564,6 +564,13 @@ prefer to use #EXT-X-START if it's in playlist instead of live_start_index.
 @item allowed_extensions
 ',' separated list of file extensions that hls is allowed to access.
 
+@item extension_picky
+This blocks disallowed extensions from probing
+It also requires all available segments to have matching extensions to the format
+except mpegts, which is always allowed.
+It is recommended to set the whitelists correctly instead of depending on extensions
+Enabled by default.
+
 @item max_reload
 Maximum number of times a insufficient list is attempted to be reloaded.
 Default value is 1000.
diff --git a/libavformat/hls.c b/libavformat/hls.c
index 045741c3b4e..93f6d1f1021 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -223,6 +223,7 @@ typedef struct HLSContext {
     AVDictionary *avio_opts;
     AVDictionary *seg_format_opts;
     char *allowed_extensions;
+    int extension_picky;
     int max_reload;
     int http_persistent;
     int http_multiple;
@@ -731,6 +732,40 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
     return ret;
 }
 
+static int test_segment(AVFormatContext *s, const AVInputFormat *in_fmt, struct playlist *pls, struct segment *seg)
+{
+    HLSContext *c = s->priv_data;
+    int matchA = 3;
+    int matchF = 0;
+
+    if (!c->extension_picky)
+        return 0;
+
+    if (strcmp(c->allowed_extensions, "ALL"))
+        matchA =      av_match_ext    (seg->url, c->allowed_extensions)
+                 + 2*(ff_match_url_ext(seg->url, c->allowed_extensions) > 0);
+
+    if (!matchA) {
+        av_log(s, AV_LOG_ERROR, "URL %s is not in allowed_extensions\n", seg->url);
+        return AVERROR_INVALIDDATA;
+    }
+
+    if (in_fmt) {
+        if (in_fmt->extensions) {
+            matchF =      av_match_ext(    seg->url, in_fmt->extensions)
+                     + 2*(ff_match_url_ext(seg->url, in_fmt->extensions) > 0);
+        } else if (!strcmp(in_fmt->name, "mpegts"))
+            matchF = 3;
+
+        if (!(matchA & matchF)) {
+            av_log(s, AV_LOG_ERROR, "detected format extension %s mismatches allowed extensions in url %s\n", in_fmt->extensions ? in_fmt->extensions : "none", seg->url);
+            return AVERROR_INVALIDDATA;
+        }
+    }
+
+    return 0;
+}
+
 static int parse_playlist(HLSContext *c, const char *url,
                           struct playlist *pls, AVIOContext *in)
 {
@@ -989,6 +1024,14 @@ static int parse_playlist(HLSContext *c, const char *url,
                     goto fail;
                 }
 
+                ret = test_segment(c->ctx, pls->ctx ? pls->ctx->iformat : NULL, pls, seg);
+                if (ret < 0) {
+                    av_free(seg->url);
+                    av_free(seg->key);
+                    av_free(seg);
+                    goto fail;
+                }
+
                 if (duration < 0.001 * AV_TIME_BASE) {
                     av_log(c->ctx, AV_LOG_WARNING, "Cannot get correct #EXTINF value of segment %s,"
                                     " set to default value to 1ms.\n", seg->url);
@@ -2114,6 +2157,11 @@ static int hls_read_header(AVFormatContext *s)
             pls->ctx->interrupt_callback = s->interrupt_callback;
             url = av_strdup(pls->segments[0]->url);
             ret = av_probe_input_buffer(&pls->pb.pub, &in_fmt, url, NULL, 0, 0);
+
+            for (int n = 0; n < pls->n_segments; n++)
+                if (ret >= 0)
+                    ret = test_segment(s, in_fmt, pls, pls->segments[n]);
+
             if (ret < 0) {
                 /* Free the ctx - it isn't initialized properly at this point,
                 * so avformat_close_input shouldn't be called. If
@@ -2576,6 +2624,8 @@ static const AVOption hls_options[] = {
         OFFSET(allowed_extensions), AV_OPT_TYPE_STRING,
         {.str = "3gp,aac,avi,ac3,eac3,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,wav"},
         INT_MIN, INT_MAX, FLAGS},
+    {"extension_picky", "Be picky with all extensions matching",
+        OFFSET(extension_picky), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS},
     {"max_reload", "Maximum number of times a insufficient list is attempted to be reloaded",
         OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 3}, 0, INT_MAX, FLAGS},
     {"m3u8_hold_counters", "The maximum number of times to load m3u8 when it refreshes without new segments",
-- 
2.48.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 2/2] avformat/hls: Be more picky on extensions

[Kieran Kunhya via ffmpeg-devel]

On Wed, 22 Jan 2025, 20:36 Michael Niedermayer, <michael@niedermayer.cc>
wrote:

> This blocks disallowed extensions from probing
> It also requires all available segments to have matching extensions to the
> format
> mpegts is treated independent of the extension
>

Potentially this is a stupid question but what stops an attacker from
faking the extension?

Kieran

>
_______________________________________________
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 2/2] avformat/hls: Be more picky on extensions

[Michael Niedermayer]

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

Hi Kieran

On Wed, Jan 22, 2025 at 10:47:52PM +0000, Kieran Kunhya via ffmpeg-devel wrote:
> On Wed, 22 Jan 2025, 20:36 Michael Niedermayer, <michael@niedermayer.cc>
> wrote:
> 
> > This blocks disallowed extensions from probing
> > It also requires all available segments to have matching extensions to the
> > format
> > mpegts is treated independent of the extension
> >
> 
> Potentially this is a stupid question but what stops an attacker from
> faking the extension?

How would he fake the extension ?

The attacker generally wants to access a sensitive file, maybe one in
/etc or maybe .ssh with something like the tty demuxer / ansi decoder

lets pick /etc/passwd as a specific example

that file does not have an extension compatible with the tty demuxer
and we assume that the attacker cannot create links on the target
because if he can he likely can also just read the file directly

This new patch should require every file now to have the appropriate
extension before reading it with that demuxer.

Before this patch you could trigger a demuxer to be probed and have
it be used on another file

If you see a way to bypass this still, please tell me

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 2/2] avformat/hls: Be more picky on extensions

[Michael Niedermayer]

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

On Wed, Jan 22, 2025 at 09:36:09PM +0100, Michael Niedermayer wrote:
> This blocks disallowed extensions from probing
> It also requires all available segments to have matching extensions to the format
> mpegts is treated independent of the extension
> 
> It is recommended to set the whitelists correctly
> instead of depending on extensions, but this should help a bit,
> and this is easier to backport
> 
> Fixes: CVE-2023-6602 II. HLS Force TTY Demuxer
> Fixes: CVE-2023-6602 IV. HLS XBIN Demuxer DoS Amplification
> 
> The other parts of CVE-2023-6602 have been fixed by prior commits
> 
> Found-by: Harvey Phillips of Amazon Element55 (element55)
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  doc/demuxers.texi |  7 +++++++
>  libavformat/hls.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+)

I intend to apply this patchset soon so it receives some testing before 7.1.1


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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato

[-- 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 2/2] avformat/hls: Be more picky on extensions

[Kieran Kunhya via ffmpeg-devel]

On Thu, 23 Jan 2025, 00:11 Michael Niedermayer, <michael@niedermayer.cc>
wrote:

> Hi Kieran
>
> On Wed, Jan 22, 2025 at 10:47:52PM +0000, Kieran Kunhya via ffmpeg-devel
> wrote:
> > On Wed, 22 Jan 2025, 20:36 Michael Niedermayer, <michael@niedermayer.cc>
> > wrote:
> >
> > > This blocks disallowed extensions from probing
> > > It also requires all available segments to have matching extensions to
> the
> > > format
> > > mpegts is treated independent of the extension
> > >
> >
> > Potentially this is a stupid question but what stops an attacker from
> > faking the extension?
>
> How would he fake the extension ?
>
> The attacker generally wants to access a sensitive file, maybe one in
> /etc or maybe .ssh with something like the tty demuxer / ansi decoder
>
> lets pick /etc/passwd as a specific example
>

Is there no control character they can use to fake the extension
potentially?

As an aside, why is this CVE from 2023 being fixed now?

Kieran

>
_______________________________________________
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 2/2] avformat/hls: Be more picky on extensions

[Michael Niedermayer]

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

Hi Kieran

On Thu, Jan 23, 2025 at 09:54:36PM +0000, Kieran Kunhya via ffmpeg-devel wrote:
> On Thu, 23 Jan 2025, 00:11 Michael Niedermayer, <michael@niedermayer.cc>
> wrote:
> 
> > Hi Kieran
> >
> > On Wed, Jan 22, 2025 at 10:47:52PM +0000, Kieran Kunhya via ffmpeg-devel
> > wrote:
> > > On Wed, 22 Jan 2025, 20:36 Michael Niedermayer, <michael@niedermayer.cc>
> > > wrote:
> > >
> > > > This blocks disallowed extensions from probing
> > > > It also requires all available segments to have matching extensions to
> > the
> > > > format
> > > > mpegts is treated independent of the extension
> > > >
> > >
> > > Potentially this is a stupid question but what stops an attacker from
> > > faking the extension?
> >
> > How would he fake the extension ?
> >
> > The attacker generally wants to access a sensitive file, maybe one in
> > /etc or maybe .ssh with something like the tty demuxer / ansi decoder
> >
> > lets pick /etc/passwd as a specific example
> >
> 
> Is there no control character they can use to fake the extension
> potentially?

If your question is, if theres a sequence of characters that gets interpreted
as an extension thats then not in the file that is being opened on one platform

Thats an interresting question, do you know of such a case ?


> 
> As an aside, why is this CVE from 2023 being fixed now?

Because it was reported now

more precissely, IIRC alexander strasser reported it after seeing it on
https://bugzilla.redhat.com/show_bug.cgi?id=2334338

I then tried to contact Harvey Phillips of Amazon Element55
and once i got in contact with him looked into fixing the
issues ffmpeg was still vulnerable to.

Yes, some CVEs out there are not reported to ffmpeg-security
at the time they should have been.

thx

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire

[-- 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 2/2] avformat/hls: Be more picky on extensions

[Michael Niedermayer]

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

On Thu, Jan 23, 2025 at 10:27:47PM +0100, Michael Niedermayer wrote:
> On Wed, Jan 22, 2025 at 09:36:09PM +0100, Michael Niedermayer wrote:
> > This blocks disallowed extensions from probing
> > It also requires all available segments to have matching extensions to the format
> > mpegts is treated independent of the extension
> > 
> > It is recommended to set the whitelists correctly
> > instead of depending on extensions, but this should help a bit,
> > and this is easier to backport
> > 
> > Fixes: CVE-2023-6602 II. HLS Force TTY Demuxer
> > Fixes: CVE-2023-6602 IV. HLS XBIN Demuxer DoS Amplification
> > 
> > The other parts of CVE-2023-6602 have been fixed by prior commits
> > 
> > Found-by: Harvey Phillips of Amazon Element55 (element55)
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  doc/demuxers.texi |  7 +++++++
> >  libavformat/hls.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 57 insertions(+)
> 
> I intend to apply this patchset soon so it receives some testing before 7.1.1

will apply


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

[-- 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 2/2] avformat/hls: Be more picky on extensions

[Vittorio Giovara]

On Sat, Jan 25, 2025 at 9:38 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Thu, Jan 23, 2025 at 10:27:47PM +0100, Michael Niedermayer wrote:
> > On Wed, Jan 22, 2025 at 09:36:09PM +0100, Michael Niedermayer wrote:
> > > This blocks disallowed extensions from probing
> > > It also requires all available segments to have matching extensions to
> the format
> > > mpegts is treated independent of the extension
> > >
> > > It is recommended to set the whitelists correctly
> > > instead of depending on extensions, but this should help a bit,
> > > and this is easier to backport
> > >
> > > Fixes: CVE-2023-6602 II. HLS Force TTY Demuxer
> > > Fixes: CVE-2023-6602 IV. HLS XBIN Demuxer DoS Amplification
> > >
> > > The other parts of CVE-2023-6602 have been fixed by prior commits
> > >
> > > Found-by: Harvey Phillips of Amazon Element55 (element55)
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  doc/demuxers.texi |  7 +++++++
> > >  libavformat/hls.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 57 insertions(+)
> >
> > I intend to apply this patchset soon so it receives some testing before
> 7.1.1
>
> will apply
>

Should this be backported to other stable releases since it's a CVE?
-- 
Vittorio
_______________________________________________
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 2/2] avformat/hls: Be more picky on extensions

[Michael Niedermayer]

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

On Tue, Jan 28, 2025 at 06:11:33AM +0100, Vittorio Giovara wrote:
> On Sat, Jan 25, 2025 at 9:38 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Thu, Jan 23, 2025 at 10:27:47PM +0100, Michael Niedermayer wrote:
> > > On Wed, Jan 22, 2025 at 09:36:09PM +0100, Michael Niedermayer wrote:
> > > > This blocks disallowed extensions from probing
> > > > It also requires all available segments to have matching extensions to
> > the format
> > > > mpegts is treated independent of the extension
> > > >
> > > > It is recommended to set the whitelists correctly
> > > > instead of depending on extensions, but this should help a bit,
> > > > and this is easier to backport
> > > >
> > > > Fixes: CVE-2023-6602 II. HLS Force TTY Demuxer
> > > > Fixes: CVE-2023-6602 IV. HLS XBIN Demuxer DoS Amplification
> > > >
> > > > The other parts of CVE-2023-6602 have been fixed by prior commits
> > > >
> > > > Found-by: Harvey Phillips of Amazon Element55 (element55)
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >  doc/demuxers.texi |  7 +++++++
> > > >  libavformat/hls.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 57 insertions(+)
> > >
> > > I intend to apply this patchset soon so it receives some testing before
> > 7.1.1
> >
> > will apply
> >
> 
> Should this be backported to other stable releases since it's a CVE?

yes, but theres a related open regression with mpv
https://trac.ffmpeg.org/ticket/11435

thx

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates

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