PR #25301 opened by Marti2203 URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25301 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25301.patch
In mov mode the name in the `hdlr` atom is a Pascal string, and its length field is 8 bits. A `handler_name` longer than 255 bytes was written with only the low 8 bits of its length, followed by the whole name. A 300-byte name is declared as 44 bytes, so a reader that honours the length gets a truncated name with garbage after it. Our own demuxer sees the mismatch and returns the name with the length byte (`,`) in front of it. This is the same class of problem as #25169 and #25186. While verifying this, I found that the demuxer side has a related bug. `mov_read_hdlr()` compares the length byte with the remaining size as a plain `char`. Where `char` is signed, a length of 128 or more never matches, so any handler name of 128 to 255 bytes reads back with its length byte as the first character. ``` ./repro.sh /path/to/ffmpeg /path/to/ffprobe # writes handler_name of 200 and 300 'A's to mov: # ffmpeg -f lavfi -i color=s=16x16:d=0.04,format=rgb24 -c:v rawvideo -metadata:s:v handler_name=AAA... out.mov # then prints the hdlr length byte and what ffprobe reads back ``` Before (c50d6fca): ``` handler_name of 200 bytes: hdlr declares 200 bytes, stores 200 bytes read back by ffprobe: 203 bytes, starts with b'\xef\xbf\xbdAAA' handler_name of 300 bytes: hdlr declares 44 bytes, stores 300 bytes read back by ffprobe: 301 bytes, starts with b',AAAAA' ``` After: ``` handler_name of 200 bytes: hdlr declares 200 bytes, stores 200 bytes read back by ffprobe: 200 bytes, starts with b'AAAAAA' handler_name of 300 bytes: hdlr declares 255 bytes, stores 255 bytes read back by ffprobe: 255 bytes, starts with b'AAAAAA' ``` The first commit truncates the name to 255 bytes with a warning, without cutting a UTF-8 sequence in half. #25186 skipped over-long metadata strings, but here I chose truncation: a handler name is always written (there is a default), and players show it as the track title, so a truncated name seems more useful than dropping it. Happy to change it to falling back to the default name instead. The second commit compares the length byte as unsigned. The third adds `fate-mov-long-handler-name`, which writes a 200-byte and a 300-byte handler name and reads them back. It is sample-free (lavfi + rawvideo), fails on c50d6fca and passes with the patches. Testing: `fate-mov`, `fate-movenc`, `fate-lavf` and `fate-source` pass (247 tests, with the needed samples). This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually. 🤖 Generated with [Claude Code](https://claude.com/claude-code) From 7e1196fa28676af007fbbd22559fcb9885a0fecf Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Wed, 21 Oct 2026 18:20:36 +0800 Subject: [PATCH 1/3] avformat/movenc: truncate handler names too long for a Pascal string In mov mode, the name in the hdlr atom is a Pascal string with an 8-bit length. For handler names longer than 255 bytes, only the low 8 bits of the length were written while the whole name was written, e.g. a 300-byte handler_name was declared as 44 bytes. Readers that honour the length get a truncated name followed by garbage in the atom, and our own demuxer, seeing a mismatched length, returns the name with the length byte (',') prepended. Truncate the name to 255 bytes, without cutting a UTF-8 sequence, and warn. Signed-off-by: Martin Mirchev <[email protected]> --- libavformat/movenc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 2c349ef663b..aba6a3c04f5 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -3731,8 +3731,18 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra avio_wb32(pb, 0); /* reserved */ avio_wb32(pb, 0); /* reserved */ descr_len = strlen(descr); - if (!track || track->mode == MODE_MOV) + if (!track || track->mode == MODE_MOV) { + /* the length of a pascal string is 8 bits */ + if (descr_len > UINT8_MAX) { + av_log(s, AV_LOG_WARNING, + "Handler name is longer than 255 bytes, truncated\n"); + descr_len = UINT8_MAX; + /* do not cut a UTF-8 sequence */ + while (descr_len && (descr[descr_len] & 0xC0) == 0x80) + descr_len--; + } avio_w8(pb, descr_len); /* pascal string */ + } avio_write(pb, descr, descr_len); /* handler description */ if (track && track->mode != MODE_MOV) avio_w8(pb, 0); /* c string */ -- 2.54.0 From a1bba15af75feb31bbe89dac00d327e8c31ae0cc Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Wed, 21 Oct 2026 18:20:45 +0800 Subject: [PATCH 2/3] avformat/mov: recognize hdlr Pascal strings of 128 to 255 bytes mov_read_hdlr() detects a QuickTime Pascal string by comparing the first byte of the name with the remaining size, but compared it as a plain char. Where char is signed, a length byte of 128 or more never matches, so the length byte is kept as the first character of handler_name, e.g. a 200-byte name written by our muxer reads back as "\xc8BBB...". Compare the length byte as unsigned. Signed-off-by: Martin Mirchev <[email protected]> --- libavformat/mov.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 910df844dce..4f9b5839884 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -853,7 +853,7 @@ static int mov_read_hdlr(MOVContext *c, AVIOContext *pb, MOVAtom atom) } title_str[title_size] = 0; if (title_str[0]) { - int off = (!c->isom && title_str[0] == title_size - 1); + int off = (!c->isom && (uint8_t)title_str[0] == title_size - 1); // flag added so as to not set stream handler name if already set from mdia->hdlr av_dict_set(&st->metadata, "handler_name", title_str + off, AV_DICT_DONT_OVERWRITE); } -- 2.54.0 From 3fcbf43b0f3fef3f45bdf8843b2b31761db5892b Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Wed, 21 Oct 2026 18:21:58 +0800 Subject: [PATCH 3/3] fate/mov: add a test for long QuickTime handler names Write a 200-byte and a 300-byte handler_name to mov and read them back: the first must be intact and the second truncated to 255 bytes. Signed-off-by: Martin Mirchev <[email protected]> --- tests/fate/mov.mak | 10 ++++++++++ tests/ref/fate/mov-long-handler-name | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 tests/ref/fate/mov-long-handler-name diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 3550e1d7b81..c5bb242b0c1 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -364,6 +364,16 @@ fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-441 fate-mov-pcm-remux: tests/data/asynth-44100-1.wav fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav -map 0 -c copy -fflags +bitexact -f mp4 fate-mov-pcm-remux: CMP = oneline + +# QuickTime hdlr names are Pascal strings with an 8-bit length: a 200-byte name +# must be read back intact and a 300-byte one must be truncated to 255 bytes. +MOV_HDLR_NAME_200 = $(subst x,0123456789,xxxxxxxxxxxxxxxxxxxx) +MOV_HDLR_NAME_300 = $(subst x,0123456789,xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, RAWVIDEO, MOV, LAVFI_INDEV COLOR_FILTER FORMAT_FILTER WRAPPED_AVFRAME_DECODER) \ + += fate-mov-long-handler-name +fate-mov-long-handler-name: CMD = transcode lavfi "color=s=16x16:d=0.04,format=rgb24" mov \ + "-map 0 -map 0 -c:v rawvideo -metadata:s:v:0 handler_name=$(MOV_HDLR_NAME_200) -metadata:s:v:1 handler_name=$(MOV_HDLR_NAME_300)" \ + "-map 0 -c copy" "-show_entries stream_tags=handler_name" fate-mov-pcm-remux: REF = 389df40a487ac8397ba20bda472434d7 FATE_MOV_FFMPEG-$(call TRANSCODE, RAWVIDEO, MOV, TESTSRC_FILTER SETPTS_FILTER) += fate-mov-vfr diff --git a/tests/ref/fate/mov-long-handler-name b/tests/ref/fate/mov-long-handler-name new file mode 100644 index 00000000000..d69b29f0720 --- /dev/null +++ b/tests/ref/fate/mov-long-handler-name @@ -0,0 +1,20 @@ +65146bb1e6c4a30d4db8afbc5cdbb61b *tests/data/fate/mov-long-handler-name.mov +3233 tests/data/fate/mov-long-handler-name.mov +#tb 0: 1/12800 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 16x16 +#sar 0: 1/1 +#tb 1: 1/12800 +#media_type 1: video +#codec_id 1: rawvideo +#dimensions 1: 16x16 +#sar 1: 1/1 +0, 0, 0, 512, 768, 0x00000000 +1, 0, 0, 512, 768, 0x00000000 +[STREAM] +TAG:handler_name=01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789 +[/STREAM] +[STREAM] +TAG:handler_name=012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234 +[/STREAM] -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
