From d6fd04486cc119638cfccd0a725434527ce51baf Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 24 Jul 2026 23:16:23 -0700 Subject: [PATCH 1/2] fix(exif): harden the shared Exif decoder against malformed input Audit of the shared Exif decoder, which every container that embeds Exif reaches with file-controlled bytes: jpeg APP1, png eXIf and the "Raw profile type exif" text chunk, heif, psd, webp, and the raw plugin's LibRaw callback. The short-blob TIFFHeader overread from the external advisory draft was already closed; the following were not, two of them crashes on an ordinary Release build with no sanitizer. Recursive IFD directory-count overread: the recursive ExifIFD/GPSIFD and Interoperability-IFD branches in read_exif_tag only checked that the target offset was < buffer size before reading a 2-byte directory count there, so an offset of buffer_size - 1 passed but the 2-byte read then ran one byte past the end of the blob (heap out-of-bounds read). Require room for the full 2-byte count before reading, matching decode_ifd, and read the count via memcpy to avoid an unaligned load (UBSan). Unknown-type size sentinel: tiff_data_size() reports an unrecognized TIFF data type as size_t(-1). dataspan(), dataptr() and tiff_dir_data() multiplied that sentinel by the entry's count and fed the result to their bounds checks, where size_t(offset) + size_t(-1) wraps to offset - 1 and passes any "> buffer size" test, yielding a span of length size_t(-1) into the blob. The trigger is not exotic: EXIF_UTF8_TYPE = 129 is the Exif 3.0 UTF-8 string type added in #4961, which add_exif_item_to_spec explicitly accepts but tiff_data_size was never taught about, so every Exif 3.0 UTF-8 tag took this path. A 26-byte blob -- one tag, type 129, count 1, offset 1 -- is a heap out-of-bounds read under ASan and an uncaught std::length_error that aborts the process in Release. tiff_data_size now knows the type (which also makes Exif 3.0 UTF-8 tags decode correctly rather than being dropped or crashing), and a new pvt::dirdata_size() rejects the sentinel so no other unknown type can wrap the arithmetic. The offset math in those helpers is promoted to 64-bit while here. IFD depth: ifd_offsets_seen stops an IFD chain from looping, but nothing stopped it from being deep -- a nested IFD only has to sit at an offset not yet seen, and 18 bytes buys one level, so a blob can chain as many levels as its own length allows and a 180 KB blob exhausts the stack. A jpeg APP1 marker caps a blob at 64 KB, but a png eXIf chunk's length field is 31 bits and heif and psd are unbounded. Now capped at 32 levels, with the depth passed down the call stack as an ordinary parameter of read_exif_tag and decode_ifd. The one wrinkle is the MakerNote, the only tag whose decoding re-enters the walk: TagInfo::HandlerFunc is public and fixed, so it cannot carry the depth. Its body moves to decode_makernote(), which takes the depth, and read_exif_tag calls that directly instead of going through the handler pointer; makernote_handler stays in the tag table as a thin wrapper so the table and tag_table() are unchanged. alloca'd rational arrays: the RATIONAL and SRATIONAL branches of add_exif_item_to_spec sized an OIIO_ALLOCA from the file's element count. OIIO_ALLOCA's own bound is a plain assert, compiled out under NDEBUG, so a Release build has no check: a 32 MB blob holding one RATIONAL tag of count 4,000,000 alloca's 16 MB and segfaults. Now OIIO_ALLOCATE_STACK_OR_HEAP, which is what the rest of the codebase uses for exactly this. Also: the directory-entry bounds test used >= where it wanted >, quietly discarding a legitimate final entry that ended exactly at the blob end, and the TIFFHeader was read through a cast that assumed alignment. Adds jpeg-corrupt and png-damaged fixtures across two container routes, built by committed generators that embed their own 1x1 carrier so they depend on no writer and carry no version-dependent metadata. Assisted-by: Claude Code / Claude Opus 4.8 Signed-off-by: Larry Gritz --- src/libOpenImageIO/exif.cpp | 85 ++++++++++++---- src/libOpenImageIO/exif.h | 35 +++++-- testsuite/jpeg-corrupt/ref/out-alt2.txt | 17 ++++ testsuite/jpeg-corrupt/ref/out-alt3.txt | 17 ++++ testsuite/jpeg-corrupt/ref/out-alt4.txt | 17 ++++ testsuite/jpeg-corrupt/ref/out-alt5.txt | 17 ++++ testsuite/jpeg-corrupt/ref/out.txt | 17 ++++ testsuite/jpeg-corrupt/run.py | 18 ++++ .../src/corrupt-exif-deep-ifds.jpg | Bin 0 -> 2158 bytes .../src/corrupt-exif-recursive-ifd.jpg | Bin 0 -> 771 bytes .../src/corrupt-exif-utf8-type.jpg | Bin 0 -> 370 bytes .../jpeg-corrupt/src/make-exif-fixtures.py | 95 ++++++++++++++++++ testsuite/png-damaged/ref/out.txt | 11 ++ testsuite/png-damaged/run.py | 7 ++ testsuite/png-damaged/src/exif-deep-ifds.png | Bin 0 -> 1893 bytes testsuite/png-damaged/src/exif-utf8-type.png | Bin 0 -> 105 bytes .../png-damaged/src/make-exif-fixtures.py | 84 ++++++++++++++++ 17 files changed, 392 insertions(+), 28 deletions(-) create mode 100644 testsuite/jpeg-corrupt/src/corrupt-exif-deep-ifds.jpg create mode 100644 testsuite/jpeg-corrupt/src/corrupt-exif-recursive-ifd.jpg create mode 100644 testsuite/jpeg-corrupt/src/corrupt-exif-utf8-type.jpg create mode 100644 testsuite/jpeg-corrupt/src/make-exif-fixtures.py create mode 100644 testsuite/png-damaged/src/exif-deep-ifds.png create mode 100644 testsuite/png-damaged/src/exif-utf8-type.png create mode 100644 testsuite/png-damaged/src/make-exif-fixtures.py diff --git a/src/libOpenImageIO/exif.cpp b/src/libOpenImageIO/exif.cpp index 9f6fc73c6a..3eeddc2907 100644 --- a/src/libOpenImageIO/exif.cpp +++ b/src/libOpenImageIO/exif.cpp @@ -161,6 +161,8 @@ tiff_data_size(TIFFDataType tifftype) static size_t sizes[] = { 0, 1, 1, 2, 4, 8, 1, 1, 2, 4, 8, 4, 8, 4 }; const int num_data_sizes = sizeof(sizes) / sizeof(*sizes); int dir_index = bitcast(tifftype); + if (dir_index == EXIF_UTF8_TYPE) + return 1; // Exif 3.0 UTF-8 string: one byte per element if (dir_index < 0 || dir_index >= num_data_sizes) { // Inform caller about corrupted entry. return -1; @@ -224,7 +226,13 @@ tiff_datatype_to_typedesc(TIFFDataType tifftype, size_t tiffcount) cspan tiff_dir_data(const TIFFDirEntry& td, cspan data) { - size_t len = tiff_data_size(td); + size_t elemsize = tiff_data_size(TIFFDataType(td.tdir_type)); + if (elemsize == 0 || elemsize == size_t(-1)) { + // Unrecognized type: we can't know how much data it has, and the + // size_t(-1) sentinel would wrap the bounds check below. + return cspan(); + } + size_t len = elemsize * size_t(td.tdir_count); if (len <= 4) { // Short data are stored in the offset field itself return cspan((const uint8_t*)&td.tdir_offset, len); @@ -365,19 +373,21 @@ version4uint8_handler(const TagInfo& taginfo, const TIFFDirEntry& dir, } +// The MakerNote is the one tag whose decoding re-enters the IFD walk, so it +// is the one that needs to know the recursion depth. TagInfo::HandlerFunc +// can't carry that, so the real work lives here and read_exif_tag calls this +// directly rather than through the handler pointer. static void -makernote_handler(const TagInfo& /*taginfo*/, const TIFFDirEntry& dir, - cspan buf, ImageSpec& spec, bool swapendian = false, - int offset_adjustment = 0) +decode_makernote(const TIFFDirEntry& dir, cspan buf, ImageSpec& spec, + bool swapendian, int offset_adjustment, int depth) { if (tiff_data_size(dir) <= 4) return; // sanity check if (spec.get_string_attribute("Make") == "Canon") { - std::vector ifdoffsets { 0 }; std::set offsets_seen; decode_ifd(buf, dir.tdir_offset, spec, pvt::canon_maker_tagmap_ref(), - offsets_seen, swapendian, offset_adjustment); + offsets_seen, swapendian, offset_adjustment, depth); } else { // Maybe we just haven't parsed the Maker metadata yet? // Allow a second try later by just stashing the maker note offset. @@ -386,6 +396,15 @@ makernote_handler(const TagInfo& /*taginfo*/, const TIFFDirEntry& dir, } +static void +makernote_handler(const TagInfo& /*taginfo*/, const TIFFDirEntry& dir, + cspan buf, ImageSpec& spec, bool swapendian = false, + int offset_adjustment = 0) +{ + decode_makernote(dir, buf, spec, swapendian, offset_adjustment, 0); +} + + static const TagInfo tiff_tag_table[] = { // clang-format off @@ -716,7 +735,10 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name, = pvt::dataspan(*dirp, buf, offset_adjustment, 2 * count); if (dspan.empty()) return; - float* f = OIIO_ALLOCA(float, count); + // The count comes from the file: bounded by the blob length, but that + // is still far too much to put on the stack. + float* f; + OIIO_ALLOCATE_STACK_OR_HEAP(f, float, count); for (size_t i = 0; i < count; ++i) { // Because the values in the blob aren't 32-bit-aligned, memcpy // them into ints to do the swapping. @@ -742,7 +764,8 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name, = pvt::dataspan(*dirp, buf, offset_adjustment, 2 * count); if (dspan.empty()) return; - float* f = OIIO_ALLOCA(float, count); + float* f; + OIIO_ALLOCATE_STACK_OR_HEAP(f, float, count); for (size_t i = 0; i < count; ++i) { // Because the values in the blob aren't 32-bit-aligned, memcpy // them into ints to do the swapping. @@ -816,14 +839,25 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name, /// integer and float data embedded in buf needs to be byte-swapped. /// Note that *dirp has not been swapped, and so is still in the native /// endianness of the file. +// An IFD may point at another IFD, and a hostile blob can chain them as deep +// as its own length allows -- each level costs a stack frame, so a few hundred +// KB of Exif is enough to exhaust the stack. Real files nest two or three +// deep. +static constexpr int max_ifd_depth = 32; + + static void read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, bool swab, int offset_adjustment, - std::set& ifd_offsets_seen, const TagMap& tagmap) + std::set& ifd_offsets_seen, const TagMap& tagmap, + int depth) { + if (depth > max_ifd_depth) + return; + if ((const uint8_t*)dirp < buf.data() || (const uint8_t*)dirp + sizeof(TIFFDirEntry) - >= buf.data() + buf.size()) { + > buf.data() + buf.size()) { #if DEBUG_EXIF_READ std::cerr << "Ignoring directory outside of the buffer.\n"; #endif @@ -859,7 +893,7 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, auto offset = unswapped_tdir_offset; // int stored in offset itself if (swab) swap_endian(&offset); - if (offset >= size_t(buf.size())) { + if (size_t(offset) + sizeof(unsigned short) > size_t(buf.size())) { #if DEBUG_EXIF_READ unsigned int off2 = offset; swap_endian(&off2); @@ -905,7 +939,8 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, read_exif_tag( spec, (const TIFFDirEntry*)(ifd + 2 + d * sizeof(TIFFDirEntry)), buf, swab, offset_adjustment, ifd_offsets_seen, - dir.tdir_tag == TIFFTAG_EXIFIFD ? exif_tagmap : gps_tagmap); + dir.tdir_tag == TIFFTAG_EXIFIFD ? exif_tagmap : gps_tagmap, + depth + 1); #if DEBUG_EXIF_READ std::cerr << "> End EXIF\n"; #endif @@ -915,7 +950,7 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, auto offset = unswapped_tdir_offset; // int stored in offset itself if (swab) swap_endian(&offset); - if (offset >= size_t(buf.size())) { + if (size_t(offset) + sizeof(unsigned short) > size_t(buf.size())) { #if DEBUG_EXIF_READ unsigned int off2 = offset; swap_endian(&off2); @@ -933,7 +968,8 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, std::cerr << "Now we've seen offset " << offset << "\n"; #endif const unsigned char* ifd = ((const unsigned char*)buf.data() + offset); - unsigned short ndirs = *(const unsigned short*)ifd; + unsigned short ndirs; + memcpy(&ndirs, ifd, sizeof(ndirs)); // hoop jumping for ubsan if (swab) swap_endian(&ndirs); #if DEBUG_EXIF_READ @@ -943,9 +979,11 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, << "\n"; #endif for (int d = 0; d < ndirs; ++d) - read_exif_tag( - spec, (const TIFFDirEntry*)(ifd + 2 + d * sizeof(TIFFDirEntry)), - buf, swab, offset_adjustment, ifd_offsets_seen, exif_tagmap); + read_exif_tag(spec, + (const TIFFDirEntry*)(ifd + 2 + + d * sizeof(TIFFDirEntry)), + buf, swab, offset_adjustment, ifd_offsets_seen, + exif_tagmap, depth + 1); #if DEBUG_EXIF_READ std::cerr << "> End Interoperability\n\n"; #endif @@ -953,7 +991,10 @@ read_exif_tag(ImageSpec& spec, const TIFFDirEntry* dirp, cspan buf, // Everything else -- use our table to handle the general case const TagInfo* taginfo = tagmap.find(dir.tdir_tag); if (taginfo && !spec.extra_attribs.contains(taginfo->name)) { - if (taginfo->handler) + if (taginfo->handler == makernote_handler) + decode_makernote(dir, buf, spec, swab, offset_adjustment, + depth + 1); + else if (taginfo->handler) taginfo->handler(*taginfo, dir, buf, spec, swab, offset_adjustment); else if (taginfo->tifftype != TIFF_NOTYPE) @@ -1094,7 +1135,7 @@ encode_exif_entry(const ParamValue& p, int tag, std::vector& dirs, bool pvt::decode_ifd(cspan buf, size_t ifd_offset, ImageSpec& spec, const TagMap& tag_map, std::set& ifd_offsets_seen, - bool swab, int offset_adjustment) + bool swab, int offset_adjustment, int depth) { // Read the directory that the header pointed to. It should contain // some number of directory entries containing tags to process. @@ -1112,7 +1153,8 @@ pvt::decode_ifd(cspan buf, size_t ifd_offset, ImageSpec& spec, for (int d = 0; d < ndirs; ++d) read_exif_tag(spec, (const TIFFDirEntry*)(ifd + 2 + d * sizeof(TIFFDirEntry)), - buf, swab, offset_adjustment, ifd_offsets_seen, tag_map); + buf, swab, offset_adjustment, ifd_offsets_seen, tag_map, + depth); return true; } @@ -1235,7 +1277,8 @@ decode_exif(cspan exif, ImageSpec& spec) // itself is also helpful in this area. if (exif.size() < sizeof(TIFFHeader)) return false; - TIFFHeader head = *(const TIFFHeader*)exif.data(); + TIFFHeader head; + memcpy(&head, exif.data(), sizeof(head)); // may be unaligned if (head.tiff_magic != 0x4949 && head.tiff_magic != 0x4d4d) return false; bool host_little = littleendian(); diff --git a/src/libOpenImageIO/exif.h b/src/libOpenImageIO/exif.h index 2789fa5b59..361cef5c27 100644 --- a/src/libOpenImageIO/exif.h +++ b/src/libOpenImageIO/exif.h @@ -31,15 +31,32 @@ namespace pvt { +// Byte length of a directory entry's data, or 0 if we can't know it. +// tiff_data_size() reports an unrecognized type as size_t(-1), which must +// never reach the bounds arithmetic below: multiplied by the count it wraps, +// and a wrapped length passes any `offset + len > size` test. +inline size_t +dirdata_size(const TIFFDirEntry& td) +{ + size_t elemsize = tiff_data_size(TIFFDataType(td.tdir_type)); + if (elemsize == 0 || elemsize == size_t(-1)) + return 0; + return elemsize * size_t(td.tdir_count); +} + + + inline const void* dataptr(const TIFFDirEntry& td, cspan data, int offset_adjustment) { - size_t len = tiff_data_size(td); + size_t len = dirdata_size(td); + if (len == 0) + return nullptr; // unknown type or no data if (len <= 4) return (const char*)&td.tdir_offset; else { - int offset = td.tdir_offset + offset_adjustment; - if (offset < 0 || size_t(offset) + len > std::size(data)) + int64_t offset = int64_t(td.tdir_offset) + offset_adjustment; + if (offset < 0 || uint64_t(offset) + len > uint64_t(std::size(data))) return nullptr; // out of bounds! return (const char*)data.data() + offset; } @@ -57,13 +74,15 @@ inline cspan dataspan(const TIFFDirEntry& td, cspan data, int offset_adjustment, size_t count) { - size_t len = tiff_data_size(td); + size_t len = dirdata_size(td); OIIO_DASSERT(len == sizeof(T) * count); + if (len == 0) + return {}; // unknown type or no data if (len <= 4) return { (const uint8_t*)&td.tdir_offset, span_size_t(len) }; else { - int offset = td.tdir_offset + offset_adjustment; - if (offset < 0 || size_t(offset) + len > std::size(data)) + int64_t offset = int64_t(td.tdir_offset) + offset_adjustment; + if (offset < 0 || uint64_t(offset) + len > uint64_t(std::size(data))) return {}; // out of bounds! return empty span return { data.data() + offset, span_size_t(len) }; } @@ -140,10 +159,12 @@ void append_tiff_dir_entry (std::vector &dirs, size_t offset_override = 0, OIIO::endian endianreq = OIIO::endian::native); +// Decode one IFD and everything it points at. `depth` is the IFD nesting +// level, incremented on each recursive step and used to bound the recursion. bool decode_ifd (cspan buf, size_t ifd_offset, ImageSpec &spec, const TagMap& tag_map, std::set& ifd_offsets_seen, bool swab=false, - int offset_adjustment=0); + int offset_adjustment=0, int depth=0); void encode_canon_makernote (std::vector& exifblob, std::vector &exifdirs, diff --git a/testsuite/jpeg-corrupt/ref/out-alt2.txt b/testsuite/jpeg-corrupt/ref/out-alt2.txt index 1d6ab6171c..c47374cc49 100644 --- a/testsuite/jpeg-corrupt/ref/out-alt2.txt +++ b/testsuite/jpeg-corrupt/ref/out-alt2.txt @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg YResolution: 300 jpeg:subsampling: "4:2:0" oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-recursive-ifd.jpg +src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg + SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D + channel list: R, G, B + jpeg:subsampling: "4:2:0" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-utf8-type.jpg +src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + ImageDescription: "" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-deep-ifds.jpg +src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + oiio:ColorSpace: "srgb_rec709_scene" corrupt-icc-4551.jpg DCT coefficient (lossy) or spatial difference (lossless) out of range Reading src/corrupt-icc-4552.jpg diff --git a/testsuite/jpeg-corrupt/ref/out-alt3.txt b/testsuite/jpeg-corrupt/ref/out-alt3.txt index 56d98f14c4..a6645903c0 100644 --- a/testsuite/jpeg-corrupt/ref/out-alt3.txt +++ b/testsuite/jpeg-corrupt/ref/out-alt3.txt @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg YResolution: 300 jpeg:subsampling: "4:2:0" oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-recursive-ifd.jpg +src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg + SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D + channel list: R, G, B + jpeg:subsampling: "4:2:0" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-utf8-type.jpg +src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + ImageDescription: "" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-deep-ifds.jpg +src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + oiio:ColorSpace: "srgb_rec709_scene" corrupt-icc-4551.jpg Reading src/corrupt-icc-4552.jpg src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg diff --git a/testsuite/jpeg-corrupt/ref/out-alt4.txt b/testsuite/jpeg-corrupt/ref/out-alt4.txt index d34a0e8b91..fc3d44696c 100644 --- a/testsuite/jpeg-corrupt/ref/out-alt4.txt +++ b/testsuite/jpeg-corrupt/ref/out-alt4.txt @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg YResolution: 300 jpeg:subsampling: "4:2:0" oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-recursive-ifd.jpg +src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg + SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D + channel list: R, G, B + jpeg:subsampling: "4:2:0" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-utf8-type.jpg +src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + ImageDescription: "" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-deep-ifds.jpg +src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + oiio:ColorSpace: "srgb_rec709_scene" corrupt-icc-4551.jpg DCT coefficient out of range Reading src/corrupt-icc-4552.jpg diff --git a/testsuite/jpeg-corrupt/ref/out-alt5.txt b/testsuite/jpeg-corrupt/ref/out-alt5.txt index fcf324fa11..1d4230ab09 100644 --- a/testsuite/jpeg-corrupt/ref/out-alt5.txt +++ b/testsuite/jpeg-corrupt/ref/out-alt5.txt @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg YResolution: 300 jpeg:subsampling: "4:2:0" oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-recursive-ifd.jpg +src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg + SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D + channel list: R, G, B + jpeg:subsampling: "4:2:0" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-utf8-type.jpg +src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + ImageDescription: "" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-deep-ifds.jpg +src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + oiio:ColorSpace: "srgb_rec709_scene" corrupt-icc-4551.jpg iconvert ERROR copying "src/corrupt-icc-4551.jpg" to "out-4551.jpg" : JPEG error: Corrupt JPEG data: bad Huffman code ("src/corrupt-icc-4551.jpg") diff --git a/testsuite/jpeg-corrupt/ref/out.txt b/testsuite/jpeg-corrupt/ref/out.txt index 0e10462c6d..c0a35b1e6e 100644 --- a/testsuite/jpeg-corrupt/ref/out.txt +++ b/testsuite/jpeg-corrupt/ref/out.txt @@ -21,6 +21,23 @@ src/corrupt-exif-1626.jpg : 256 x 256, 3 channel, uint8 jpeg YResolution: 300 jpeg:subsampling: "4:2:0" oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-recursive-ifd.jpg +src/corrupt-exif-recursive-ifd.jpg : 1 x 1, 3 channel, uint8 jpeg + SHA-1: 29E2DCFBB16F63BB0254DF7585A15BB6FB5E927D + channel list: R, G, B + jpeg:subsampling: "4:2:0" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-utf8-type.jpg +src/corrupt-exif-utf8-type.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + ImageDescription: "" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/corrupt-exif-deep-ifds.jpg +src/corrupt-exif-deep-ifds.jpg : 1 x 1, 1 channel, uint8 jpeg + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + oiio:ColorSpace: "srgb_rec709_scene" corrupt-icc-4551.jpg DCT coefficient (lossy) or spatial difference (lossless) out of range Reading src/corrupt-icc-4552.jpg diff --git a/testsuite/jpeg-corrupt/run.py b/testsuite/jpeg-corrupt/run.py index a2ae4becbe..09c0973fc4 100755 --- a/testsuite/jpeg-corrupt/run.py +++ b/testsuite/jpeg-corrupt/run.py @@ -21,6 +21,24 @@ # nonsensical length, that before being fixed, caused a buffer overrun. command += info_command ("src/corrupt-exif-1626.jpg", safematch=True) +# This file's Exif block has an ExifIFD pointer whose offset lands one byte +# before the end of the Exif buffer. The shared decoder's recursive IFD branch +# read a 2-byte directory count there; before being fixed it ran one byte past +# the buffer (heap OOB read). Must now be dropped cleanly. +command += info_command ("src/corrupt-exif-recursive-ifd.jpg", safematch=True) + +# This file's Exif block declares a tag with TIFF data type 129, the Exif 3.0 +# UTF-8 type. tiff_data_size() didn't know that type and reported size_t(-1), +# which wrapped the shared decoder's bounds arithmetic into a span of length +# size_t(-1): a heap OOB read under a sanitizer, and an uncaught +# std::length_error that aborted the process on a release build. +command += info_command ("src/corrupt-exif-utf8-type.jpg", safematch=True) + +# This file's Exif block chains 100 ExifIFD pointers, each aimed at the next. +# Every level costs a stack frame, so a large enough chain exhausts the stack; +# the decoder now stops descending after a fixed depth. +command += info_command ("src/corrupt-exif-deep-ifds.jpg", safematch=True) + # This file has a corrupted ICC profile block that has tags that say they # extend beyond the boundaries of the ICC block itself. command += run_app (oiiotool("--echo corrupt-icc-4551.jpg")) diff --git a/testsuite/jpeg-corrupt/src/corrupt-exif-deep-ifds.jpg b/testsuite/jpeg-corrupt/src/corrupt-exif-deep-ifds.jpg new file mode 100644 index 0000000000000000000000000000000000000000..9b9caedfd23bd54a2b6b593ce59c926c56e1226a GIT binary patch literal 2158 zcmZY9eN0Vp7{Kx0IrrR`Zktt-H1nFInN3KV3BBmvnGr%WBT3iHNRl>2QZav!B>iEN zB+VaQDrsgUNi*{jDrOZzGfP66InL>>#r;0#oM-3vIluFqJNkZ6c zLBb-0kc1;g(XtSJ+NH;kOm)IPG7qv8k^o7CWJ3xe<&c|@N02s14@A-1)nceqA4ni1 z46+!q4zeAR3CV|)LaHH+kk^ne5Fxwv?gjCOgg{~-@sO>MeUM|2Vn_v~9?}fyg8YUI zGq_G;93&VL35kOwLoy&oA!i_$A+?YuNGGHZVpUvw9}SrXnGcDDBtp_4xsW2rCCDwv zV@L<27h*EH_U;P_g3N_1fvkt5K(ZhOkTOUOKiq+ckAxe-*0%(`1D!R^A|6hTUy&Y-oAU^ g`QhWIuCG1cdcXhp+4rkoZ50-MY~ex`;r(jMUsXf>10ztB5hMlF zm4eXa1yti-kecV2o0y*J>91g{XQXGKXOxm!reLMuXkcOHY++$w;_PDLXl88bY~*Tb zZfR<6V&Q0MY;I&=Z0P3bY+-H=G}e%jiNOhI7RVJK@c%Z0Gtf>Hzy#)jNG2FyW@cdq z0VXC8g9QYbm|0m_Sb&Iy1!M@+1fVV^7G|I-lEME241ydWuQM|$F)#@-G7B>PKf)jn z3=LL@*$@W<`9RZI+1NQaxwwG}whAyXF#;XN0(K5iwiYPQz#_;hq-f~KCLEZ^u2d*u z)Hrb=hqBYggQ7tfKd2Zd6*X~kiHS={N~x-;YiMejn3|beSXw!|xVpJ}czOkggocGj zL`Eg2q^6~3WM&nYl$MoOR8}>&w6?W(baqXeJZ0*%=`&`|TC{k{(q+q6tX#Ee^OmjK zw(r=v>(JpNM~@vpaq`rq%U7;myME*5t%r{uKY9A>`HPpYK7RWA=tD+&BkYgZwVxh2-Q6qGeQ$1KQT&+wnY{+Gt02K)av E0SA@X`2YX_ literal 0 HcmV?d00001 diff --git a/testsuite/jpeg-corrupt/src/corrupt-exif-utf8-type.jpg b/testsuite/jpeg-corrupt/src/corrupt-exif-utf8-type.jpg new file mode 100644 index 0000000000000000000000000000000000000000..9d240736a1f496bb26e4fe5e4260267f2305a59b GIT binary patch literal 370 zcmex=k|Jw}C zKpqNU0`ov56AUmjvoM1I6BCHR0s>6TtgI|7K*YlG{{RCw2gp1|MnQ)EM;PRRhO(iUwW$pkka<)WpdpCN3cY31zV>gMj@=@lFj8WtWA8I_!p znwFlCnN?g;T2@|BS=HRq+ScCD*)?hMl&RCE&zL!D(c&dbmn~nha@D5ITefc7zGLUE zLx+zXJ$C%W$y1juU%7hi`i+~n9zJ^fH', len(body) + 2) + body + + +def tiff_header(diroff=8): + return b'II' + struct.pack(' 1 else '.') diff --git a/testsuite/png-damaged/ref/out.txt b/testsuite/png-damaged/ref/out.txt index 4998469ba9..d2e8c98972 100644 --- a/testsuite/png-damaged/ref/out.txt +++ b/testsuite/png-damaged/ref/out.txt @@ -11,3 +11,14 @@ idiff ERROR: Could not read invalid_gray_alpha_sbit.png: PNG read error: oFFs: Read error: hit end of file in png reader Invalid image file "invalid_gray_alpha_sbit.png": Read error: hit end of file in png reader PNG read error: oFFs: Read error: hit end of file in png reader +Reading src/exif-utf8-type.png +src/exif-utf8-type.png : 1 x 1, 1 channel, uint8 png + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + ImageDescription: "" + oiio:ColorSpace: "srgb_rec709_scene" +Reading src/exif-deep-ifds.png +src/exif-deep-ifds.png : 1 x 1, 1 channel, uint8 png + SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F + channel list: Y + oiio:ColorSpace: "srgb_rec709_scene" diff --git a/testsuite/png-damaged/run.py b/testsuite/png-damaged/run.py index 4db5c18586..1d003886a0 100755 --- a/testsuite/png-damaged/run.py +++ b/testsuite/png-damaged/run.py @@ -12,3 +12,10 @@ command += rw_command (OIIO_TESTSUITE_IMAGEDIR + "/broken", "invalid_gray_alpha_sbit.png", printinfo=False) + +# These carry malformed Exif payloads in a PNG eXIf chunk, reaching the same +# shared decoder as the .jpg fixtures in testsuite/jpeg-corrupt/src. The eXIf +# length field is 31 bits, so this is the route where a deeply nested IFD +# chain can be made large enough to exhaust the stack. +command += info_command ("src/exif-utf8-type.png", safematch=True) +command += info_command ("src/exif-deep-ifds.png", safematch=True) diff --git a/testsuite/png-damaged/src/exif-deep-ifds.png b/testsuite/png-damaged/src/exif-deep-ifds.png new file mode 100644 index 0000000000000000000000000000000000000000..86b860869a2b3fca183b4f8c6ed1df9bb37b3ea6 GIT binary patch literal 1893 zcmZY5PbfrD6vy%Vo-w0WBT17c3kyk;EG(o+8a$aNP1A%VY4R^h8j@r&X_7x>AxT0O znj{GeNs@&kNwTnzBw1KUk}M?2eP%Iw-&6P8)2FlfovC1;#;LhAA%xTL>mm6o=1Y@J zc%w@VLO5KVO-7etc!Y_H=$Wwn?YPYqSxG5Oz&|O61R-sZLC82{0kQ!(gj_-%Any>H zc@kO4q%KGS#0#l|v_c|~7-SBz2HA(4L+&825TRPLXF~EIB@jQP8PW@hLS`VVkX^_r z<3u%G$ zK}I37kT_%yat67Dyg+^+8IHs?65aPzbs{It&@1YPm)pZ~R)yl E1=KyGqyPW_ literal 0 HcmV?d00001 diff --git a/testsuite/png-damaged/src/exif-utf8-type.png b/testsuite/png-damaged/src/exif-utf8-type.png new file mode 100644 index 0000000000000000000000000000000000000000..63ea003c2a4c92773ac33a70ddf831b4019f3cb0 GIT binary patch literal 105 zcmeAS@N?(olHy`uVBq!ia0vp^j3CSbBp9sfW`_bPsniJ1G*3@029O{l10Q1}L=8v@ p5X_r;PZLOSdAc};RLn_E0J51F7(Lc6DFm_@JYD@<);T3K0RR?=5vu?I literal 0 HcmV?d00001 diff --git a/testsuite/png-damaged/src/make-exif-fixtures.py b/testsuite/png-damaged/src/make-exif-fixtures.py new file mode 100644 index 0000000000..0afe69e980 --- /dev/null +++ b/testsuite/png-damaged/src/make-exif-fixtures.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 + +# Copyright Contributors to the OpenImageIO project. +# SPDX-License-Identifier: Apache-2.0 +# https://github.com/AcademySoftwareFoundation/OpenImageIO + +"""Generator for the malformed-Exif .png fixtures in this directory. + +The files it writes are committed, so this only needs to be run if they must +be regenerated: + + python3 make-exif-fixtures.py . + +These carry the same hand-built Exif payloads as the .jpg fixtures in +testsuite/jpeg-corrupt/src, but through PNG's eXIf chunk instead of JPEG's +APP1 marker. The two containers reach the same shared decoder, and the PNG +route matters on its own because an eXIf chunk's length field is 31 bits -- +a JPEG APP1 marker caps out at 64 KB, so PNG is where a deeply nested IFD +chain can be made large enough to exhaust the stack. + + exif-utf8-type.png A tag whose TIFF data type is 129, the Exif 3.0 UTF-8 + type, which tiff_data_size() reported as size_t(-1). + exif-deep-ifds.png A chain of ExifIFD pointers, each aimed at the next. +""" + +import binascii +import os +import struct +import sys +import zlib + + +def chunk(kind, data): + return (struct.pack('>I', len(data)) + kind + data + + struct.pack('>I', binascii.crc32(kind + data) & 0xffffffff)) + + +def png(exif_payload): + # 1x1 8-bit grayscale, one zlib-compressed scanline (filter byte + pixel). + ihdr = struct.pack('>IIBBBBB', 1, 1, 8, 0, 0, 0, 0) + idat = zlib.compress(b'\0\0') + return (b'\x89PNG\r\n\x1a\n' + chunk(b'IHDR', ihdr) + + chunk(b'eXIf', exif_payload) + chunk(b'IDAT', idat) + + chunk(b'IEND', b'')) + + +def tiff_header(diroff=8): + return b'II' + struct.pack(' 1 else '.') From 5094c78f020082fec759b36846e5c3169de4ddfc Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 14 Aug 2026 17:58:06 -0700 Subject: [PATCH 2/2] Also fix tiff_datatype_to_typedesc to be aware of EXIF_UTF8_TYPE Signed-off-by: Larry Gritz --- src/libOpenImageIO/exif.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libOpenImageIO/exif.cpp b/src/libOpenImageIO/exif.cpp index 3eeddc2907..5f61a81cba 100644 --- a/src/libOpenImageIO/exif.cpp +++ b/src/libOpenImageIO/exif.cpp @@ -218,6 +218,8 @@ tiff_datatype_to_typedesc(TIFFDataType tifftype, size_t tiffcount) #endif default: break; } + if (static_cast(tifftype) == EXIF_UTF8_TYPE) + return TypeString; return TypeUnknown; }