From 44620ade251f705b1395d91a540c51768b2d9c8b Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Sun, 18 Feb 2024 15:19:49 +0100 Subject: [PATCH] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free 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). Reviewed-by: Anton Khirnov Signed-off-by: Andreas Rheinhardt --- 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;