[FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free

* [FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free
@ 2024-02-18 15:10 Andreas Rheinhardt
  2024-02-18 15:10 ` [FFmpeg-devel] [PATCH 2/2] tests/fate-run: Do not ignore errors from intermediate commands Andreas Rheinhardt
  2024-02-18 16:57 ` [FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free Anton Khirnov
  0 siblings, 2 replies; 3+ messages in thread
From: Andreas Rheinhardt @ 2024-02-18 15:10 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

The filename is freed with the OptionsContext and therefore
there will be a use-after-free when reporting the filename
in print_stream_maps(). So create a copy of the string.

This is a regression since 8aed3911fc454e79697e183660bf30d31334a64b.
fate-lavf-mkv_attachment exhibits it (and reports a random nonsense
filename here), but this does not make the test fail (not even with
valgrind; only with ASAN, as it aborts on use-after-free).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 fftools/ffmpeg.h          |  2 +-
 fftools/ffmpeg_mux.c      |  2 ++
 fftools/ffmpeg_mux_init.c | 10 +++++++++-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 33750e0bb3..c394f60962 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -555,7 +555,7 @@ typedef struct OutputStream {
     AVDictionary *swr_opts;
     char *apad;
 
-    const char *attachment_filename;
+    char *attachment_filename;
 
     int keep_pix_fmt;
 
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index e65fe89992..5a648c0568 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -817,6 +817,8 @@ static void ost_free(OutputStream **post)
     av_freep(&ost->logfile_prefix);
     av_freep(&ost->apad);
 
+    av_freep(&ost->attachment_filename);
+
 #if FFMPEG_OPT_MAP_CHANNEL
     av_freep(&ost->audio_channels_map);
     ost->audio_channels_mapped = 0;
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 0718487c53..1abbb2d945 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1741,6 +1741,7 @@ static int of_add_attachments(Muxer *mux, const OptionsContext *o)
     for (int i = 0; i < o->nb_attachments; i++) {
         AVIOContext *pb;
         uint8_t *attachment;
+        char *attachment_filename;
         const char *p;
         int64_t len;
 
@@ -1788,13 +1789,20 @@ read_fail:
         av_log(mux, AV_LOG_VERBOSE, "Creating attachment stream from file %s\n",
                o->attachments[i]);
 
+        attachment_filename = av_strdup(o->attachments[i]);
+        if (!attachment_filename) {
+            av_free(attachment);
+            return AVERROR(ENOMEM);
+        }
+
         err = ost_add(mux, o, AVMEDIA_TYPE_ATTACHMENT, NULL, NULL, &ost);
         if (err < 0) {
+            av_free(attachment_filename);
             av_freep(&attachment);
             return err;
         }
 
-        ost->attachment_filename       = o->attachments[i];
+        ost->attachment_filename       = attachment_filename;
         ost->par_in->extradata         = attachment;
         ost->par_in->extradata_size    = len;
 
-- 
2.34.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".

^ permalink raw reply	[flat|nested] 3+ messages in thread