Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PR] fftools/ffmpeg_demux: Check metadata provided filename and add av_safe_filename() (PR #21803)
@ 2026-02-19 18:53 michaelni via ffmpeg-devel
  0 siblings, 0 replies; only message in thread
From: michaelni via ffmpeg-devel @ 2026-02-19 18:53 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: michaelni

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 <mxu490469@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>


>From ed66e82d4d41f1c7a4e9538fe25dba969487a73c Mon Sep 17 00:00:00 2001
From: Michael Niedermayer <michael@niedermayer.cc>
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 <mxu490469@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 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 <michael@niedermayer.cc>
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 <michael@niedermayer.cc>
---
 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 -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-02-19 18:53 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-19 18:53 [FFmpeg-devel] [PR] fftools/ffmpeg_demux: Check metadata provided filename and add av_safe_filename() (PR #21803) michaelni via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git