PR #21258 opened by ruikai URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21258 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21258.patch
The extra-IFD feature stores trailing IFDs as dummy AV_TIFF_IFD entries. If extraction stops early (e.g. non-contiguous tags or invalid type/count), one of these entries can remain in the IFD while the output size is computed as if it had no directory slot. This can make exif_write_ifd() write past the end of the buffer. Scan the full reserved tag range, validate candidates as single AV_TIFF_IFD entries, and allocate the output buffer after any successful extraction so all directory entries are counted. Repro (ASan): ``` ./configure --toolchain=clang-asan --enable-debug \ --disable-optimizations make -j"$(nproc)" ASAN_OPTIONS=halt_on_error=1:detect_leaks=0 ./ffmpeg_g \ -loglevel error -nostdin -i poc.png -f null - ``` This triggers an OOB write in exif.c:731 (exif_write_ifd). Reference: https://gist.github.com/retr0reg/bc5f5dd9e2afedb09853913f1d1ee246 Regression: 784aa09fa8 Found-by: Ruikai Peng, Pwno >From d621a482740cd3827b0f42d0f424d1c234e42b2c Mon Sep 17 00:00:00 2001 From: Ruikai Peng <[email protected]> Date: Sun, 21 Dec 2025 21:21:01 -0500 Subject: [PATCH] avcodec/exif: size buffer after extra IFD extraction The extra-IFD feature stores trailing IFDs as dummy AV_TIFF_IFD entries. If extraction stops early (e.g. non-contiguous tags or invalid type/count), one of these entries can remain in the IFD while the output size is computed as if it had no directory slot. This can make exif_write_ifd() write past the end of the buffer. Scan the full reserved tag range, validate candidates as single AV_TIFF_IFD entries, and allocate the output buffer after any successful extraction so all directory entries are counted. Repro (ASan): ./configure --toolchain=clang-asan --enable-debug \ --disable-optimizations make -j"$(nproc)" ASAN_OPTIONS=halt_on_error=1:detect_leaks=0 ./ffmpeg_g \ -loglevel error -nostdin -i poc.png -f null - This triggers an OOB write in exif.c:731 (exif_write_ifd). Reference: https://gist.github.com/retr0reg/bc5f5dd9e2afedb09853913f1d1ee246 Regression: 784aa09fa8 Found-by: Ruikai Peng, Pwno --- libavcodec/exif.c | 69 ++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/libavcodec/exif.c b/libavcodec/exif.c index 0de543e35a..b79644ac91 100644 --- a/libavcodec/exif.c +++ b/libavcodec/exif.c @@ -673,9 +673,7 @@ static size_t exif_get_ifd_size(const AVExifMetadata *ifd) for (size_t i = 0; i < ifd->count; i++) { const AVExifEntry *entry = &ifd->entries[i]; if (entry->type == AV_TIFF_IFD) { - /* this is an extra IFD, not an entry, so we don't need to add base tag size */ - size_t base_size = entry->id > 0xFFECu && entry->id <= 0xFFFCu ? 0 : BASE_TAG_SIZE; - total_size += base_size + exif_get_ifd_size(&entry->value.ifd) + entry->ifd_offset; + total_size += BASE_TAG_SIZE + exif_get_ifd_size(&entry->value.ifd) + entry->ifd_offset; } else { size_t payload_size = entry->count * exif_sizes[entry->type]; total_size += BASE_TAG_SIZE + (payload_size > 4 ? payload_size : 0); @@ -759,7 +757,6 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer, goto end; } - size = exif_get_ifd_size(ifd); switch (header_mode) { case AV_EXIF_EXIF00: off = 6; @@ -776,6 +773,44 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer, headsize = 0; break; } + + int extras = 0; + for (uint16_t extra_tag = 0xFFFCu; + extra_tag > 0xFFECu && extras < FF_ARRAY_ELEMS(extra_ifds); + extra_tag--) { + AVExifEntry *extra_entry = NULL; + + ret = av_exif_get_entry(logctx, (AVExifMetadata *) ifd, extra_tag, 0, &extra_entry); + if (ret <= 0) + continue; + av_log(logctx, AV_LOG_DEBUG, "found extra IFD tag: %04x\n", extra_tag); + if (extra_entry->type != AV_TIFF_IFD || extra_entry->count != 1) { + av_log(logctx, AV_LOG_DEBUG, "invalid extra IFD tag: %04x\n", extra_tag); + continue; + } + if (!ifd_new) { + ifd_new = av_exif_clone_ifd(ifd); + if (!ifd_new) + break; + ifd = ifd_new; + } + /* calling remove_entry will call av_exif_free on the original */ + AVExifMetadata *cloned = av_exif_clone_ifd(&extra_entry->value.ifd); + if (!cloned) + break; + extra_ifds[extras] = *cloned; + /* don't use av_exif_free here, we want to preserve internals */ + av_free(cloned); + ret = av_exif_remove_entry(logctx, ifd_new, extra_tag, 0); + if (ret < 0) + break; + + extras++; + } + + size = exif_get_ifd_size(ifd); + for (int i = 0; i < extras; i++) + size += exif_get_ifd_size(&extra_ifds[i]); buf = av_buffer_alloc(size + off + headsize); if (!buf) { ret = AVERROR(ENOMEM); @@ -798,32 +833,6 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer, tput32(&pb, le, 8); } - int extras; - for (extras = 0; extras < FF_ARRAY_ELEMS(extra_ifds); extras++) { - AVExifEntry *extra_entry = NULL; - uint16_t extra_tag = 0xFFFCu - extras; - ret = av_exif_get_entry(logctx, (AVExifMetadata *) ifd, extra_tag, 0, &extra_entry); - if (ret <= 0) - break; - av_log(logctx, AV_LOG_DEBUG, "found extra IFD tag: %04x\n", extra_tag); - if (!ifd_new) { - ifd_new = av_exif_clone_ifd(ifd); - if (!ifd_new) - break; - ifd = ifd_new; - } - /* calling remove_entry will call av_exif_free on the original */ - AVExifMetadata *cloned = av_exif_clone_ifd(&extra_entry->value.ifd); - if (!cloned) - break; - extra_ifds[extras] = *cloned; - /* don't use av_exif_free here, we want to preserve internals */ - av_free(cloned); - ret = av_exif_remove_entry(logctx, ifd_new, extra_tag, 0); - if (ret < 0) - break; - } - next = bytestream2_tell_p(&pb); ret = exif_write_ifd(logctx, &pb, le, 0, ifd); if (ret < 0) { -- 2.49.1 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
