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

[Michael Niedermayer <michael@niedermayer.cc>]

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

Hi James

On Fri, Jan 17, 2025 at 07:37:16PM -0300, James Almer wrote:
> On 1/16/2025 12:39 AM, Michael Niedermayer wrote:
> > This blocks disallowed extensions from probing
> > It also requires segments to have matching extensions to the format
> > 
> > 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>
> > ---
> >   libavformat/hls.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 045741c3b4e..a802eafc3fe 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;
> > @@ -2114,6 +2115,24 @@ 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);
> > +            if (c->extension_picky && ret >= 0) {
> 
> This should be a check for s->strict_std_compliance instead of a new demuxer
> specific option, IMO.
> Since you want the strict behavior enabled by default, make this line be:
> 
> s->strict_std_compliance >= FF_COMPLIANCE_NORMAL && ret >= 0

I have not attempted to implement any standard compliance

This is an implementation of security checks to avoid the quoted issues

and even if, by chance, my implementation happens to be what a standard
demands.
overloading strict_std_compliance with a security feature is not correct

The question "do i want "bitstream level" standard compliance"?
and
The question "am i sure the input is fully trusted so i can turn of
a security feature" / do i understand what i do and have i taken precautions
like setting up whitelists

target different users and different use cases

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