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

[James Almer]

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 at 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

> +                for (int n = 0; n < pls->n_segments; n++) {
> +                    struct segment *seg = pls->segments[n];
> +                    if ( strcmp(c->allowed_extensions, "ALL") &&
> +                        !av_match_ext    (seg->url, c->allowed_extensions) &&
> +                        !ff_match_url_ext(seg->url, c->allowed_extensions)) {
> +                        av_log(s, AV_LOG_ERROR, "URL %s is not in allowed_extensions\n", seg->url);
> +                        ret = AVERROR_INVALIDDATA;
> +                    }
> +
> +                    if (!in_fmt->extensions ||
> +                        !av_match_ext(    seg->url, in_fmt->extensions) &&
> +                        !ff_match_url_ext(seg->url, in_fmt->extensions)) {
> +                        av_log(s, AV_LOG_ERROR, "detected format extension %s mismatches url %s\n", in_fmt->extensions ? in_fmt->extensions : "none", seg->url);
> +                        ret = AVERROR_INVALIDDATA;
> +                    }
> +                }
> +            }
>               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 +2595,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",

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250117/1bf4979a/attachment.sig>