PR #21299 opened by Leo Izen (Traneptora) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21299 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21299.patch
Maliciously constructed input EXIF blocks could use the extra IFDs we set aside in noncontinugous ways, which will cause them to be written as subdirectories of IFD0 rather than existing IFDs. The base tag size is (correctly) excluded from the size calculation, but if they are being written as additional tags, the allocation will be too small and a write may overflow. Signed-off-by: Leo Izen <[email protected]> >From 9febe411cff9d280ec316008bb616ba971311fdb Mon Sep 17 00:00:00 2001 From: Leo Izen <[email protected]> Date: Thu, 25 Dec 2025 15:49:05 -0500 Subject: [PATCH] avcodec/exif.c: avoid buffer overflow with extra IFDs Maliciously constructed input EXIF blocks could use the extra IFDs we set aside in noncontinugous ways, which will cause them to be written as subdirectories of IFD0 rather than existing IFDs. The base tag size is (correctly) excluded from the size calculation, but if they are being written as additional tags, the allocation will be too small and a write may overflow. Signed-off-by: Leo Izen <[email protected]> --- libavcodec/exif.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/libavcodec/exif.c b/libavcodec/exif.c index 0de543e35a..5ce402784c 100644 --- a/libavcodec/exif.c +++ b/libavcodec/exif.c @@ -798,13 +798,15 @@ 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++) { + int extras = 0; + for (int i = 0; i < FF_ARRAY_ELEMS(extra_ifds); i++) { AVExifEntry *extra_entry = NULL; - uint16_t extra_tag = 0xFFFCu - extras; + uint16_t extra_tag = 0xFFFCu - i; ret = av_exif_get_entry(logctx, (AVExifMetadata *) ifd, extra_tag, 0, &extra_entry); - if (ret <= 0) + if (ret < 0) break; + if (!ret) + continue; av_log(logctx, AV_LOG_DEBUG, "found extra IFD tag: %04x\n", extra_tag); if (!ifd_new) { ifd_new = av_exif_clone_ifd(ifd); @@ -816,7 +818,7 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer, AVExifMetadata *cloned = av_exif_clone_ifd(&extra_entry->value.ifd); if (!cloned) break; - extra_ifds[extras] = *cloned; + 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); @@ -824,12 +826,16 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer, break; } + if (ret < 0) { + av_log(logctx, AV_LOG_ERROR, "error popping additional IFD: %s\n", av_err2str(ret)); + goto end; + } + next = bytestream2_tell_p(&pb); ret = exif_write_ifd(logctx, &pb, le, 0, ifd); if (ret < 0) { - av_buffer_unref(&buf); av_log(logctx, AV_LOG_ERROR, "error writing EXIF data: %s\n", av_err2str(ret)); - return ret; + goto end; } next += ret; @@ -840,8 +846,10 @@ int av_exif_write(void *logctx, const AVExifMetadata *ifd, AVBufferRef **buffer, tput32(&pb, le, next); bytestream2_seek_p(&pb, next, SEEK_SET); ret = exif_write_ifd(logctx, &pb, le, 0, &extra_ifds[i]); - if (ret < 0) - break; + if (ret < 0) { + av_log(logctx, AV_LOG_ERROR, "error writing additional IFD: %s\n", av_err2str(ret)); + goto end; + } next += ret; } @@ -853,6 +861,8 @@ end: av_freep(&ifd_new); for (int i = 0; i < FF_ARRAY_ELEMS(extra_ifds); i++) av_exif_free(&extra_ifds[i]); + if (ret < 0) + av_buffer_unref(&buf); return ret; } -- 2.49.1 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
