PR #21597 opened by kingroryg URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21597 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21597.patch
The vtt_basename variable, derived from user-provided output filename concatenated with "%d.vtt", was used directly as a format string in snprintf(). If the user provides a filename containing format specifiers (e.g., "output_%s_test.m3u8"), these specifiers are interpreted by snprintf(), causing stack memory to be read. This could lead to: - Information disclosure via %x, %p, %s reading stack memory - Denial of service via %s reading invalid pointers - Potential code execution via %n (if not blocked by system) Fix by using %.*s to safely copy the base portion of the filename without interpreting any format specifiers it may contain. Add unit test to verify format string safety. Reproduction steps for the vuln: 1. Create a test video with subtitles: ```bash ffmpeg -f lavfi -i "color=c=red:s=64x64:d=1" -f lavfi -i "sine=f=440:d=1" \ -c:v mpeg4 -c:a aac -shortest test_input.mp4 echo -e "1\n00:00:00,000 --> 00:00:01,000\nTest" > test.srt ``` 2. Create initial HLS playlist with format specifiers in filename: ```bash ffmpeg -i test_input.mp4 -i test.srt -c:v copy -c:a copy -c:s webvtt \ -f hls "test_%s_INJECT.m3u8" ``` 3. Trigger vulnerable code path with `append_list`: ```bash ffmpeg -i test_input.mp4 -i test.srt -c:v copy -c:a copy -c:s webvtt \ -f hls -hls_flags append_list "test_%s_INJECT.m3u8" ``` Signed-off-by: Sarthak Munshi <[email protected]> >From 197d073bcd0cf97caed20f2db941077facbe0e73 Mon Sep 17 00:00:00 2001 From: Sarthak Munshi <[email protected]> Date: Mon, 26 Jan 2026 19:05:17 -0800 Subject: [PATCH] avformat/hlsenc: fix format string vulnerability in parse_playlist The vtt_basename variable, derived from user-provided output filename concatenated with "%d.vtt", was used directly as a format string in snprintf(). If the user provides a filename containing format specifiers (e.g., "output_%s_test.m3u8"), these specifiers are interpreted by snprintf(), causing stack memory to be read. This could lead to: - Information disclosure via %x, %p, %s reading stack memory - Denial of service via %s reading invalid pointers - Potential code execution via %n (if not blocked by system) Fix by using %.*s to safely copy the base portion of the filename without interpreting any format specifiers it may contain. Add unit test to verify format string safety. Signed-off-by: Sarthak Munshi <[email protected]> --- libavformat/Makefile | 1 + libavformat/hlsenc.c | 7 +++- libavformat/tests/hlsenc.c | 79 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 libavformat/tests/hlsenc.c diff --git a/libavformat/Makefile b/libavformat/Makefile index 5fd3f7252a..848610d2aa 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -782,6 +782,7 @@ TESTPROGS-$(CONFIG_MOV_MUXER) += movenc TESTPROGS-$(CONFIG_NETWORK) += noproxy TESTPROGS-$(CONFIG_SRTP) += srtp TESTPROGS-$(CONFIG_IMF_DEMUXER) += imf +TESTPROGS-$(CONFIG_HLS_MUXER) += hlsenc TOOLS = aviocat \ ismindex \ diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 7105404d1e..5996a52218 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -1242,13 +1242,16 @@ static int parse_playlist(AVFormatContext *s, const char *url, VariantStream *vs if (vs->has_subtitle) { int vtt_index = extract_segment_number(line); const char *vtt_basename = av_basename(vs->vtt_basename); - int len = strlen(vtt_basename) + 11; + const char *fmt = strstr(vtt_basename, "%d"); + int base_len = fmt ? (int)(fmt - vtt_basename) : strlen(vtt_basename); + int len = base_len + 25; char *vtt_file = av_mallocz(len); if (!vtt_file) { ret = AVERROR(ENOMEM); goto fail; } - snprintf(vtt_file, len, vtt_basename, vtt_index); + /* Use %.*s to safely copy base without format string interpretation */ + snprintf(vtt_file, len, "%.*s%d.vtt", base_len, vtt_basename, vtt_index); ff_format_set_url(vs->vtt_avf, vtt_file); } diff --git a/libavformat/tests/hlsenc.c b/libavformat/tests/hlsenc.c new file mode 100644 index 0000000000..181ae0de47 --- /dev/null +++ b/libavformat/tests/hlsenc.c @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2025 FFmpeg developers + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stdio.h> +#include <string.h> + +static int test_vtt_filename_fmtstr(void) +{ + int ret = 0; + struct { + const char *vtt_basename; + int vtt_index; + const char *expected; + } tests[] = { + { "normal%d.vtt", 5, "normal5.vtt" }, + { "test_%s_file%d.vtt", 10, "test_%s_file10.vtt" }, + { "leak%x%x%d.vtt", 0, "leak%x%x0.vtt" }, + { "%p%n%d.vtt", 1, "%p%n1.vtt" }, + { "safe_name%d.vtt", 123, "safe_name123.vtt" }, + }; + int i; + + printf("Testing VTT filename format string safety:\n"); + + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) { + const char *vtt_basename = tests[i].vtt_basename; + int vtt_index = tests[i].vtt_index; + const char *expected = tests[i].expected; + const char *fmt; + int base_len, len; + char vtt_file[256]; + + fmt = strstr(vtt_basename, "%d"); + base_len = fmt ? (int)(fmt - vtt_basename) : strlen(vtt_basename); + len = base_len + 25; + + if (len > sizeof(vtt_file)) + len = sizeof(vtt_file); + + snprintf(vtt_file, len, "%.*s%d.vtt", base_len, vtt_basename, vtt_index); + + if (strcmp(vtt_file, expected) != 0) { + printf(" FAIL: input='%s' idx=%d => '%s' (expected '%s')\n", + vtt_basename, vtt_index, vtt_file, expected); + ret = 1; + } else { + printf(" PASS: input='%s' idx=%d => '%s'\n", + vtt_basename, vtt_index, vtt_file); + } + } + + return ret; +} + +int main(void) +{ + int ret = 0; + + ret |= test_vtt_filename_fmtstr(); + + return ret; +} -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
