[FFmpeg-devel] [PATCH] avcodec/exif: fix some coverity errors (PR #20296)

[FFmpeg-devel] [PATCH] avcodec/exif: fix some coverity errors (PR #20296)

[Leo Izen via ffmpeg-devel]

PR #20296 opened by Leo Izen (Traneptora)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20296
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20296.patch

This commit fixes some memory and security issues due to improper
sanitizing and checks.

Fixes:
 - CID 1665100..1665107

Signed-off-by: Leo Izen <leo.izen@gmail.com>
Reportedy-by: James Almer <jamrial@gmail.com>


From 954fcecf4aa582eee5d93561fd85a8dcba218683 Mon Sep 17 00:00:00 2001
From: Leo Izen <leo.izen@gmail.com>
Date: Wed, 20 Aug 2025 21:40:28 -0400
Subject: [PATCH] avcodec/exif: fix some coverity errors

This commit fixes some memory and security issues due to improper
sanitizing and checks.

Fixes:
 - CID 1665100..1665107

Signed-off-by: Leo Izen <leo.izen@gmail.com>
Reportedy-by: James Almer <jamrial@gmail.com>
---
 libavcodec/exif.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/libavcodec/exif.c b/libavcodec/exif.c
index 48959eb9b3..2513ee4ec0 100644
--- a/libavcodec/exif.c
+++ b/libavcodec/exif.c
@@ -322,12 +322,17 @@ static int exif_read_values(void *logctx, GetByteContext *gb, int le, AVExifEntr
             break;
         case AV_TIFF_UNDEFINED:
         case AV_TIFF_BYTE:
+            /* these three fields are aliased to entry->value.ptr via a union */
+            /* and entry->value.ptr will always be nonzero here */
+            av_assert0(entry->value.ubytes);
             bytestream2_get_buffer(gb, entry->value.ubytes, entry->count);
             break;
         case AV_TIFF_SBYTE:
+            av_assert0(entry->value.sbytes);
             bytestream2_get_buffer(gb, entry->value.sbytes, entry->count);
             break;
         case AV_TIFF_STRING:
+            av_assert0(entry->value.str);
             bytestream2_get_buffer(gb, entry->value.str, entry->count);
             break;
     }
@@ -471,6 +476,10 @@ static int exif_decode_tag(void *logctx, GetByteContext *gb, int le,
     av_log(logctx, AV_LOG_DEBUG, "TIFF Tag: id: 0x%04x, type: %d, count: %u, offset: %d, "
                                  "payload: %" PRIu32 "\n", entry->id, type, count, tell, payload);
 
+    /* AV_TIFF_IFD is the largest, numerically */
+    if (type > AV_TIFF_IFD)
+        return AVERROR_INVALIDDATA;
+
     is_ifd = type == AV_TIFF_IFD || ff_tis_ifd(entry->id) || entry->id == MAKERNOTE_TAG;
 
     if (is_ifd) {
@@ -541,6 +550,11 @@ static int exif_parse_ifd_list(void *logctx, GetByteContext *gb, int le,
         av_log(logctx, AV_LOG_ERROR, "not enough bytes remaining in EXIF buffer. entries: %" PRIu32 "\n", entries);
         return AVERROR_INVALIDDATA;
     }
+    if (entries > 4096) {
+        /* that is a lot of entries, probably an error */
+        av_log(logctx, AV_LOG_ERROR, "too many entries: %" PRIu32 "\n", entries);
+        return AVERROR_INVALIDDATA;
+    }
 
     ifd->count = entries;
     av_log(logctx, AV_LOG_DEBUG, "entry count for IFD: %u\n", ifd->count);
@@ -729,7 +743,12 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer,
 
     if (header_mode != AV_EXIF_ASSUME_BE && header_mode != AV_EXIF_ASSUME_LE) {
         /* these constants are be32 in both cases */
-        bytestream2_put_be32(&pb, le ? EXIF_II_LONG : EXIF_MM_LONG);
+        /* this is a #if instead of a ternary to suppress a deadcode warning */
+#if AV_HAVE_BIGENDIAN
+        bytestream2_put_be32(&pb, EXIF_MM_LONG);
+#else
+        bytestream2_put_be32(&pb, EXIF_II_LONG);
+#endif
         tput32(&pb, le, 8);
     }
 
@@ -897,10 +916,10 @@ static int exif_ifd_to_dict(void *logctx, const char *prefix, const AVExifMetada
             if (ret < 0)
                 goto end;
             ret = av_dict_set(metadata, key, value, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
-            if (ret < 0)
-                goto end;
             key = NULL;
             value = NULL;
+            if (ret < 0)
+                goto end;
         } else {
             av_freep(&key);
         }
@@ -970,7 +989,7 @@ static int exif_attach_ifd(void *logctx, AVFrame *frame, const AVExifMetadata *i
 
     ret = av_exif_ifd_to_dict(logctx, cloned ? cloned : ifd, &frame->metadata);
     if (ret < 0)
-        return ret;
+        goto end;
 
     if (cloned || !og) {
         ret = av_exif_write(logctx, cloned ? cloned : ifd, &written, AV_EXIF_TIFF_HEADER);
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".