PR #21803 opened by michaelni URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21803 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21803.patch
Fixes: path traversal with -dump_attachment:t Fixes: malicious.mkv Based on code from libavformat/concatdec.c This will be factored out into libavutil in the next commit, It is here for easy backport without API change Found-by: Shangzhi Xu <[email protected]> Signed-off-by: Michael Niedermayer <[email protected]> >From ed66e82d4d41f1c7a4e9538fe25dba969487a73c Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 19 Feb 2026 18:14:28 +0100 Subject: [PATCH 1/2] fftools/ffmpeg_demux: Check metadata provided filename Fixes: path traversal with -dump_attachment:t Fixes: malicious.mkv Based on code from libavformat/concatdec.c This will be factored out into libavutil in the next commit, It is here for easy backport without API change Found-by: Shangzhi Xu <[email protected]> Signed-off-by: Michael Niedermayer <[email protected]> --- fftools/ffmpeg_demux.c | 46 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index c8d8a7e044..939e55fad6 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1748,6 +1748,45 @@ static int istg_add(const OptionsContext *o, Demuxer *d, AVStreamGroup *stg) return 0; } +static int is_windows_reserved_device_name(const char *f) +{ + char stem[6], *s; + av_strlcpy(stem, f, sizeof(stem)); + if((s=strchr(stem, '.'))) + *s = 0; + if((s=strpbrk(stem, "123456789"))) + *s = '1'; + + return !av_strcasecmp(stem, "AUX") || + !av_strcasecmp(stem, "CON") || + !av_strcasecmp(stem, "NUL") || + !av_strcasecmp(stem, "PRN") || + !av_strcasecmp(stem, "COM1") || + !av_strcasecmp(stem, "LPT1"); +} + +static int safe_filename(const char *f, int allow_subdir) +{ + const char *start = f; + + if (!*f || is_windows_reserved_device_name(f)) + return 0; + + for (; *f; f++) { + /* A-Za-z0-9_- */ + if (!((unsigned)((*f | 32) - 'a') < 26 || + (unsigned)(*f - '0') < 10 || *f == '_' || *f == '-')) { + if (f == start) + return 0; + else if (allow_subdir && *f == '/') + start = f + 1; + else if (*f != '.') + return 0; + } + } + return 1; +} + static int dump_attachment(InputStream *ist, const char *filename) { AVStream *st = ist->st; @@ -1759,8 +1798,13 @@ static int dump_attachment(InputStream *ist, const char *filename) av_log(ist, AV_LOG_WARNING, "No extradata to dump.\n"); return 0; } - if (!*filename && (e = av_dict_get(st->metadata, "filename", NULL, 0))) + if (!*filename && (e = av_dict_get(st->metadata, "filename", NULL, 0))) { filename = e->value; + if (!safe_filename(filename, 0)) { + av_log(ist, AV_LOG_ERROR, "Filemame %s is unsafe\n", filename); + return AVERROR(EINVAL); + } + } if (!*filename) { av_log(ist, AV_LOG_FATAL, "No filename specified and no 'filename' tag"); return AVERROR(EINVAL); -- 2.52.0 >From 38fd15464dbc8017d5b3e4ae132f0179cc20081b Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 19 Feb 2026 19:36:51 +0100 Subject: [PATCH 2/2] avutil/avstring: Factor code into av_safe_filename() Signed-off-by: Michael Niedermayer <[email protected]> --- doc/APIchanges | 3 +++ fftools/ffmpeg_demux.c | 41 +---------------------------------------- libavformat/concatdec.c | 21 +-------------------- libavutil/avstring.c | 39 +++++++++++++++++++++++++++++++++++++++ libavutil/avstring.h | 7 +++++++ 5 files changed, 51 insertions(+), 60 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index 2b43139b48..5beff45af8 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2025-03-28 API changes, most recent first: +2026-02-xx - xxxxxxxxxx - lavu 60.xx.100 - avstring.h + add av_safe_filename() + 2026-02-13 - xxxxxxxxxx - lavu 60.25.100 - avassert.h Deprecate av_assert0_fpu() and av_assert2_fpu() without replacement. diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 939e55fad6..19bc4bc10a 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1748,45 +1748,6 @@ static int istg_add(const OptionsContext *o, Demuxer *d, AVStreamGroup *stg) return 0; } -static int is_windows_reserved_device_name(const char *f) -{ - char stem[6], *s; - av_strlcpy(stem, f, sizeof(stem)); - if((s=strchr(stem, '.'))) - *s = 0; - if((s=strpbrk(stem, "123456789"))) - *s = '1'; - - return !av_strcasecmp(stem, "AUX") || - !av_strcasecmp(stem, "CON") || - !av_strcasecmp(stem, "NUL") || - !av_strcasecmp(stem, "PRN") || - !av_strcasecmp(stem, "COM1") || - !av_strcasecmp(stem, "LPT1"); -} - -static int safe_filename(const char *f, int allow_subdir) -{ - const char *start = f; - - if (!*f || is_windows_reserved_device_name(f)) - return 0; - - for (; *f; f++) { - /* A-Za-z0-9_- */ - if (!((unsigned)((*f | 32) - 'a') < 26 || - (unsigned)(*f - '0') < 10 || *f == '_' || *f == '-')) { - if (f == start) - return 0; - else if (allow_subdir && *f == '/') - start = f + 1; - else if (*f != '.') - return 0; - } - } - return 1; -} - static int dump_attachment(InputStream *ist, const char *filename) { AVStream *st = ist->st; @@ -1800,7 +1761,7 @@ static int dump_attachment(InputStream *ist, const char *filename) } if (!*filename && (e = av_dict_get(st->metadata, "filename", NULL, 0))) { filename = e->value; - if (!safe_filename(filename, 0)) { + if (av_safe_filename(filename, 0) <= 0) { av_log(ist, AV_LOG_ERROR, "Filemame %s is unsafe\n", filename); return AVERROR(EINVAL); } diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index e0c2c87248..cc19c160fb 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -91,25 +91,6 @@ static char *get_keyword(uint8_t **cursor) return ret; } -static int safe_filename(const char *f) -{ - const char *start = f; - - for (; *f; f++) { - /* A-Za-z0-9_- */ - if (!((unsigned)((*f | 32) - 'a') < 26 || - (unsigned)(*f - '0') < 10 || *f == '_' || *f == '-')) { - if (f == start) - return 0; - else if (*f == '/') - start = f + 1; - else if (*f != '.') - return 0; - } - } - return 1; -} - #define FAIL(retcode) do { ret = (retcode); goto fail; } while(0) static int add_file(AVFormatContext *avf, char *filename, ConcatFile **rfile, @@ -123,7 +104,7 @@ static int add_file(AVFormatContext *avf, char *filename, ConcatFile **rfile, size_t url_len; int ret; - if (cat->safe && !safe_filename(filename)) { + if (cat->safe && av_safe_filename(filename, 1) <= 0) { av_log(avf, AV_LOG_ERROR, "Unsafe file name '%s'\n", filename); FAIL(AVERROR(EPERM)); } diff --git a/libavutil/avstring.c b/libavutil/avstring.c index 281c5cdc88..41c0e9751b 100644 --- a/libavutil/avstring.c +++ b/libavutil/avstring.c @@ -463,3 +463,42 @@ int av_match_list(const char *name, const char *list, char separator) return 0; } + +static int is_windows_reserved_device_name(const char *f) +{ + char stem[6], *s; + av_strlcpy(stem, f, sizeof(stem)); + if((s=strchr(stem, '.'))) + *s = 0; + if((s=strpbrk(stem, "123456789"))) + *s = '1'; + + return !av_strcasecmp(stem, "AUX") || + !av_strcasecmp(stem, "CON") || + !av_strcasecmp(stem, "NUL") || + !av_strcasecmp(stem, "PRN") || + !av_strcasecmp(stem, "COM1") || + !av_strcasecmp(stem, "LPT1"); +} + +int av_safe_filename(const char *f, int allow_subdir) +{ + const char *start = f; + + if (!*f || is_windows_reserved_device_name(f)) + return 0; + + for (; *f; f++) { + /* A-Za-z0-9_- */ + if (!((unsigned)((*f | 32) - 'a') < 26 || + (unsigned)(*f - '0') < 10 || *f == '_' || *f == '-')) { + if (f == start) + return 0; + else if (allow_subdir && *f == '/') + start = f + 1; + else if (*f != '.') + return 0; + } + } + return 1; +} diff --git a/libavutil/avstring.h b/libavutil/avstring.h index 17f7b03db5..12219401e8 100644 --- a/libavutil/avstring.h +++ b/libavutil/avstring.h @@ -421,6 +421,13 @@ int av_match_list(const char *name, const char *list, char separator); */ int av_sscanf(const char *string, const char *format, ...) av_scanf_format(2, 3); +/** + * Check if the given filename is safe. + * @param allow_subdir 1 means allowing subdirectories, 0 means allowing only files in the current directory + * @returns positive if safe, 0 if unsafe, negative on error (the current implementation does not return errors, future implementations might) + */ +int av_safe_filename(const char *f, int allow_subdir); + /** * @} */ -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
