PR #21322 opened by Marton Balint (cus) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21322 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21322.patch
Previous code truncated chapter numbers more than 999 and and time offsets more than 99 hours. The Vorbis chapter extension only allows 1000 chapters, but there is a trivial way to extend the syntax, so let's do that instead. Alternate would be to reject it or only write the first 1000 chapters. In any case, it is better than the current code which writes truncated entries. This also fixes the GCC warnings for truncated strings. >From 27df50b1b31422191c4ae7f1d174dc3fa3d816ce Mon Sep 17 00:00:00 2001 From: Marton Balint <[email protected]> Date: Thu, 25 Dec 2025 20:03:32 +0100 Subject: [PATCH 1/3] avformat/aviobuf: return error for ffio_close_null_buf() if written bytes exceed INT_MAX Also check return value where it is used. Signed-off-by: Marton Balint <[email protected]> --- libavformat/avio_internal.h | 2 +- libavformat/aviobuf.c | 4 +++- libavformat/movenc.c | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/libavformat/avio_internal.h b/libavformat/avio_internal.h index 7d4756db0c..fadf19dae5 100644 --- a/libavformat/avio_internal.h +++ b/libavformat/avio_internal.h @@ -259,7 +259,7 @@ int ffio_open_whitelist(AVIOContext **s, const char *url, int flags, * Close a null buffer. * * @param s an IO context opened by ffio_open_null_buf - * @return the number of bytes written to the null buffer + * @return the number of bytes written to the null buffer, negative on error */ int ffio_close_null_buf(AVIOContext *s); diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 9beac8bcd5..373a48eea5 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -1451,6 +1451,8 @@ static int null_buf_write(void *opaque, const uint8_t *buf, int buf_size) { DynBuffer *d = opaque; + if ((unsigned)d->pos + (unsigned)buf_size > INT_MAX) + return AVERROR(ERANGE); d->pos += buf_size; if (d->pos > d->size) d->size = d->pos; @@ -1474,7 +1476,7 @@ int ffio_close_null_buf(AVIOContext *s) avio_flush(s); - size = d->size; + size = s->error ? s->error : d->size; avio_context_free(&s); diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 8d8acd2aff..e844be483c 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -5845,8 +5845,11 @@ static int mov_write_sidx_tags(AVIOContext *pb, MOVMuxContext *mov, total_size -= mov_write_sidx_tag(avio_buf, track, ref_size, total_size); } - if (round == 0) + if (round == 0) { total_size = ffio_close_null_buf(avio_buf); + if (total_size < 0) + return total_size; + } } return 0; } @@ -5911,6 +5914,8 @@ static int mov_write_moof_tag(AVIOContext *pb, MOVMuxContext *mov, int tracks, return ret; mov_write_moof_tag_internal(avio_buf, mov, tracks, 0); moof_size = ffio_close_null_buf(avio_buf); + if (moof_size < 0) + return moof_size; if (mov->flags & FF_MOV_FLAG_DASH && !(mov->flags & (FF_MOV_FLAG_GLOBAL_SIDX | FF_MOV_FLAG_SKIP_SIDX))) -- 2.49.1 >From bc662906eda8fb7af81b9d96d2f3701ac35d5ca5 Mon Sep 17 00:00:00 2001 From: Marton Balint <[email protected]> Date: Tue, 23 Dec 2025 02:52:59 +0100 Subject: [PATCH 2/3] avformat/vorbiscomment: use null buf to calculate vorbis comment length Also check possible failures when calculating length, and change return type to int as bigger return values are no longer possible. --- libavformat/flacenc.c | 4 +++- libavformat/matroskaenc.c | 4 +++- libavformat/oggenc.c | 5 ++++- libavformat/vorbiscomment.c | 35 +++++++++++++++-------------------- libavformat/vorbiscomment.h | 4 ++-- 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c index a8beec7750..711119efec 100644 --- a/libavformat/flacenc.c +++ b/libavformat/flacenc.c @@ -64,11 +64,13 @@ static int flac_write_block_comment(AVIOContext *pb, AVDictionary **m, int last_block, int bitexact) { const char *vendor = bitexact ? "ffmpeg" : LIBAVFORMAT_IDENT; - int64_t len; + int len; ff_metadata_conv(m, ff_vorbiscomment_metadata_conv, NULL); len = ff_vorbiscomment_length(*m, vendor, NULL, 0); + if (len < 0) + return len; if (len >= ((1<<24) - 4)) return AVERROR(EINVAL); diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 18f17f4329..50188c396c 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1061,12 +1061,14 @@ static int put_flac_codecpriv(AVFormatContext *s, AVIOContext *pb, "Lavf" : LIBAVFORMAT_IDENT; AVDictionary *dict = NULL; uint8_t buf[32]; - int64_t len; + int len; snprintf(buf, sizeof(buf), "0x%"PRIx64, par->ch_layout.u.mask); av_dict_set(&dict, "WAVEFORMATEXTENSIBLE_CHANNEL_MASK", buf, 0); len = ff_vorbiscomment_length(dict, vendor, NULL, 0); + if (len < 0) + return len; av_assert1(len < (1 << 24) - 4); avio_w8(pb, 0x84); diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c index 9a548a8d29..be85386a6a 100644 --- a/libavformat/oggenc.c +++ b/libavformat/oggenc.c @@ -295,7 +295,10 @@ static uint8_t *ogg_write_vorbiscomment(int64_t offset, int bitexact, ff_metadata_conv(m, ff_vorbiscomment_metadata_conv, NULL); - size = offset + ff_vorbiscomment_length(*m, vendor, chapters, nb_chapters) + framing_bit; + size = ff_vorbiscomment_length(*m, vendor, chapters, nb_chapters); + if (size < 0) + return NULL; + size += offset + framing_bit; if (size > INT_MAX) return NULL; p = av_mallocz(size); diff --git a/libavformat/vorbiscomment.c b/libavformat/vorbiscomment.c index abe12fd586..fede6b68de 100644 --- a/libavformat/vorbiscomment.c +++ b/libavformat/vorbiscomment.c @@ -20,6 +20,7 @@ */ #include "avio.h" +#include "avio_internal.h" #include "avformat.h" #include "metadata.h" #include "vorbiscomment.h" @@ -38,27 +39,21 @@ const AVMetadataConv ff_vorbiscomment_metadata_conv[] = { { 0 } }; -int64_t ff_vorbiscomment_length(const AVDictionary *m, const char *vendor_string, - AVChapter **chapters, unsigned int nb_chapters) +int ff_vorbiscomment_length(const AVDictionary *m, const char *vendor_string, + AVChapter **chapters, unsigned int nb_chapters) { - int64_t len = 8; - len += strlen(vendor_string); - if (chapters && nb_chapters) { - for (int i = 0; i < nb_chapters; i++) { - const AVDictionaryEntry *tag = NULL; - len += 4 + 12 + 1 + 10; - while ((tag = av_dict_iterate(chapters[i]->metadata, tag))) { - int64_t len1 = !strcmp(tag->key, "title") ? 4 : strlen(tag->key); - len += 4 + 10 + len1 + 1 + strlen(tag->value); - } - } - } - if (m) { - const AVDictionaryEntry *tag = NULL; - while ((tag = av_dict_iterate(m, tag))) { - len += 4 +strlen(tag->key) + 1 + strlen(tag->value); - } - } + AVIOContext *avio_buf; + int ret, len; + + ret = ffio_open_null_buf(&avio_buf); + if (ret < 0) + return ret; + + ret = ff_vorbiscomment_write(avio_buf, m, vendor_string, chapters, nb_chapters); + len = ffio_close_null_buf(avio_buf); + if (ret < 0) + return ret; + return len; } diff --git a/libavformat/vorbiscomment.h b/libavformat/vorbiscomment.h index 7cacd0b2a0..cd8b325fb6 100644 --- a/libavformat/vorbiscomment.h +++ b/libavformat/vorbiscomment.h @@ -34,8 +34,8 @@ * For no string, set to an empty string. * @return The length in bytes. */ -int64_t ff_vorbiscomment_length(const AVDictionary *m, const char *vendor_string, - AVChapter **chapters, unsigned int nb_chapters); +int ff_vorbiscomment_length(const AVDictionary *m, const char *vendor_string, + AVChapter **chapters, unsigned int nb_chapters); /** * Write a VorbisComment into an AVIOContext. The output size can be obtained -- 2.49.1 >From d859b6014b5b1439e360eaa8ba0a875ceeaf33c4 Mon Sep 17 00:00:00 2001 From: Marton Balint <[email protected]> Date: Thu, 25 Dec 2025 19:20:51 +0100 Subject: [PATCH 3/3] avformat/vorbiscomment: fix writing huge chapter numbers and time offets to vorbiscomment Previous code truncated chapter numbers more than 999 and and time offsets more than 99 hours. The Vorbis chapter extension only allows 1000 chapters, but there is a trivial way to extend the syntax, so let's do that instead. Alternate would be to reject it or only write the first 1000 chapters. In any case, it is better than the current code which writes truncated entries. This also fixes the GCC warnings for truncated strings. Signed-off-by: Marton Balint <[email protected]> --- libavformat/vorbiscomment.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/libavformat/vorbiscomment.c b/libavformat/vorbiscomment.c index fede6b68de..8d846f5c1b 100644 --- a/libavformat/vorbiscomment.c +++ b/libavformat/vorbiscomment.c @@ -24,6 +24,7 @@ #include "avformat.h" #include "metadata.h" #include "vorbiscomment.h" +#include "libavutil/bprint.h" #include "libavutil/dict.h" /** @@ -86,8 +87,7 @@ int ff_vorbiscomment_write(AVIOContext *pb, const AVDictionary *m, } for (int i = 0; i < nb_chapters; i++) { AVChapter *chp = chapters[i]; - char chapter_time[13]; - char chapter_number[4]; + AVBPrint bp; int h, m, s, ms; s = av_rescale(chp->start, chp->time_base.num, chp->time_base.den); @@ -95,23 +95,21 @@ int ff_vorbiscomment_write(AVIOContext *pb, const AVDictionary *m, m = (s / 60) % 60; ms = av_rescale_q(chp->start, chp->time_base, av_make_q( 1, 1000)) % 1000; s = s % 60; - snprintf(chapter_number, sizeof(chapter_number), "%03d", i); - snprintf(chapter_time, sizeof(chapter_time), "%02d:%02d:%02d.%03d", h, m, s, ms); - avio_wl32(pb, 10 + 1 + 12); - avio_write(pb, "CHAPTER", 7); - avio_write(pb, chapter_number, 3); - avio_w8(pb, '='); - avio_write(pb, chapter_time, 12); + av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); + av_bprintf(&bp, "CHAPTER%03d=%02d:%02d:%02d.%03d", i, h, m, s, ms); + avio_wl32(pb, bp.len); + avio_write(pb, bp.str, bp.len); tag = NULL; while ((tag = av_dict_iterate(chapters[i]->metadata, tag))) { int64_t len1 = !strcmp(tag->key, "title") ? 4 : strlen(tag->key); int64_t len2 = strlen(tag->value); - if (len1+1+len2+10 > UINT32_MAX) + av_bprint_clear(&bp); + av_bprintf(&bp, "CHAPTER%03d", i); + if (len1+1+len2+bp.len > UINT32_MAX) return AVERROR(EINVAL); - avio_wl32(pb, 10 + len1 + 1 + len2); - avio_write(pb, "CHAPTER", 7); - avio_write(pb, chapter_number, 3); + avio_wl32(pb, bp.len + len1 + 1 + len2); + avio_printf(pb, bp.str, bp.len); if (!strcmp(tag->key, "title")) avio_write(pb, "NAME", 4); else -- 2.49.1 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
