[FFmpeg-devel] [PATCH] avcodec/exif: size buffer after extra IFD extraction (PR #21258)

[ruikai via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

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 <ruikai@pwno.io>
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 -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org