PR #25409 opened by Daniel Campos Ramos (danielcamposramos) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25409 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25409.patch
Prepared with AI assistance through Codex. The outer H.264 SEI parser adds extension bytes to a signed payload type and an unsigned payload size without checking the sums. A sufficiently long type header overflows the signed accumulator, while a size header can wrap to zero and pass the payload-length check. Check each addition against INT_MAX using the actual next byte, so a representable terminal sum is still accepted. The FATE reference covers ordinary and extended unknown messages, the largest signed type, type and size overflow, unsigned size wrap, a truncated payload and an unterminated header. This came out of the frame-packing persistence work for Sparky Stereo OS. The two overflow probes come from the review of #24628; this issue predates that series and is proposed separately. Validation: GCC 16.2 and clang 21.1.8 each pass all 6,077 applicable FATE targets. Both overflow probes fail before the fix and pass afterward; the valid INT_MAX boundary also passes. >From cea40a340bf0dcf6715608ca2057957048f20444 Mon Sep 17 00:00:00 2001 From: Daniel Campos Ramos <[email protected]> Date: Fri, 23 Oct 2026 04:05:29 -0300 Subject: [PATCH] avcodec/h264_sei: bound payload header accumulation Reject header additions before the payload type or size exceeds INT_MAX. Compare the limit with the next byte so representable terminal sums remain valid. Synthetic type overflow and unsigned size wrap headers were accepted before this change. Add a FATE reference covering those failures, the maximum signed type, ordinary extended headers and truncation. Signed-off-by: Daniel Campos Ramos <[email protected]> Assisted-by: gpt-6-astra via Codex --- libavcodec/Makefile | 1 + libavcodec/h264_sei.c | 6 ++- libavcodec/tests/h264_sei.c | 80 ++++++++++++++++++++++++++++++++++ tests/fate/libavcodec.mak | 4 ++ tests/ref/fate/h264-sei-header | 8 ++++ 5 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 libavcodec/tests/h264_sei.c create mode 100644 tests/ref/fate/h264-sei-header diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 53c638c8780..15704073257 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -1397,6 +1397,7 @@ TESTPROGS-$(CONFIG_APV_DECODER) += apv TESTPROGS-$(CONFIG_AV1_VAAPI_ENCODER) += av1_levels TESTPROGS-$(CONFIG_CABAC) += cabac TESTPROGS-$(CONFIG_CELP_MATH) += celp_math +TESTPROGS-$(CONFIG_H264_SEI) += h264_sei TESTPROGS-$(CONFIG_GOLOMB) += golomb TESTPROGS-$(CONFIG_IDCTDSP) += dct TESTPROGS-$(CONFIG_DXV_ENCODER) += hashtable diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c index 6819fb75c40..8defa9a771d 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -244,13 +244,15 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, int ret = 0; do { - if (bytestream2_get_bytes_left(&gbyte) <= 0) + if (bytestream2_get_bytes_left(&gbyte) <= 0 || + type > INT_MAX - bytestream2_peek_byteu(&gbyte)) return AVERROR_INVALIDDATA; type += bytestream2_peek_byteu(&gbyte); } while (bytestream2_get_byteu(&gbyte) == 255); do { - if (bytestream2_get_bytes_left(&gbyte) <= 0) + if (bytestream2_get_bytes_left(&gbyte) <= 0 || + size > INT_MAX - bytestream2_peek_byteu(&gbyte)) return AVERROR_INVALIDDATA; size += bytestream2_peek_byteu(&gbyte); } while (bytestream2_get_byteu(&gbyte) == 255); diff --git a/libavcodec/tests/h264_sei.c b/libavcodec/tests/h264_sei.c new file mode 100644 index 00000000000..4fe4049d4d2 --- /dev/null +++ b/libavcodec/tests/h264_sei.c @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2026 Daniel Campos Ramos + * + * 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 <limits.h> +#include <stdint.h> +#include <stdio.h> +#include <string.h> + +#include "libavutil/log.h" +#include "libavutil/mem.h" +#include "libavcodec/h264_sei.h" + +static uint8_t *put_header(uint8_t *p, uint64_t value) +{ + size_t count = value / 255; + + memset(p, 255, count); + p += count; + *p++ = value % 255; + return p; +} + +static int check(const char *name, uint64_t type, uint64_t size, + int truncate) +{ + size_t capacity = type / 255 + size / 255 + 3; + uint8_t *buf = av_mallocz(capacity + AV_INPUT_BUFFER_PADDING_SIZE); + H264SEIContext sei = { 0 }; + H264ParamSets ps = { 0 }; + GetBitContext gb; + uint8_t *p; + int ret; + + if (!buf) + return 1; + p = put_header(buf, type); + p = put_header(p, size); + *p++ = 0x80; + if (truncate) { + memset(buf, 255, 3); + p = buf + 3; + } + ret = init_get_bits8(&gb, buf, p - buf); + if (ret >= 0) + ret = ff_h264_sei_decode(&sei, &gb, &ps, NULL); + printf("%s: %d\n", name, ret); + ff_h264_sei_uninit(&sei); + av_free(buf); + return 0; +} + +int main(void) +{ + av_log_set_level(AV_LOG_QUIET); + return check("empty unknown message", 200, 0, 0) || + check("extended type", 511, 0, 0) || + check("maximum signed type", INT_MAX, 0, 0) || + check("type overflow", (uint64_t)INT_MAX + 1, 0, 0) || + check("size overflow", 200, (uint64_t)INT_MAX + 1, 0) || + check("size wrap", 200, (uint64_t)UINT_MAX + 1, 0) || + check("truncated payload", 200, 2, 0) || + check("unterminated type", 200, 0, 1); +} diff --git a/tests/fate/libavcodec.mak b/tests/fate/libavcodec.mak index 713b1feb368..ccb16b68873 100644 --- a/tests/fate/libavcodec.mak +++ b/tests/fate/libavcodec.mak @@ -134,5 +134,9 @@ $(FATE_SAMPLES_ENC_RECON-yes): CMP = null FATE_EXTERN-yes += $(FATE_SAMPLES_ENC_RECON-yes) fate-enc-recon: $(FATE_SAMPLES_ENC_RECON-yes) +FATE_LIBAVCODEC-$(CONFIG_H264_SEI) += fate-h264-sei-header +fate-h264-sei-header: libavcodec/tests/h264_sei$(EXESUF) +fate-h264-sei-header: CMD = run libavcodec/tests/h264_sei$(EXESUF) + FATE-$(CONFIG_AVCODEC) += $(FATE_LIBAVCODEC-yes) fate-libavcodec: $(FATE_LIBAVCODEC-yes) diff --git a/tests/ref/fate/h264-sei-header b/tests/ref/fate/h264-sei-header new file mode 100644 index 00000000000..5d7534f1f1a --- /dev/null +++ b/tests/ref/fate/h264-sei-header @@ -0,0 +1,8 @@ +empty unknown message: 0 +extended type: 0 +maximum signed type: 0 +type overflow: -1094995529 +size overflow: -1094995529 +size wrap: -1094995529 +truncated payload: -1094995529 +unterminated type: -1094995529 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
