From 120463804bd7e638bfa8861c3d026cd18934947f Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 26 Jun 2026 09:24:48 -0400 Subject: [PATCH 1/2] fix(iff): detect corrupt chunk sizes, flags, channel configs Detect and rejecct corrupt chunk sizes, flags, and channel configurations in the header. Assisted-by: Claude Code / Claude Opus 4.8 Signed-off-by: Larry Gritz --- src/iff.imageio/iffinput.cpp | 55 +++++++++++++++++----- testsuite/iff/ref/out.txt | 8 ++++ testsuite/iff/run.py | 6 ++- testsuite/iff/src/bad_rgba_chunk_size.iff | Bin 0 -> 646 bytes 4 files changed, 55 insertions(+), 14 deletions(-) create mode 100644 testsuite/iff/src/bad_rgba_chunk_size.iff diff --git a/src/iff.imageio/iffinput.cpp b/src/iff.imageio/iffinput.cpp index 3686f538f5..50ed29e4d5 100644 --- a/src/iff.imageio/iffinput.cpp +++ b/src/iff.imageio/iffinput.cpp @@ -90,8 +90,6 @@ class IffInput final : public ImageInput { return read_type_len(name, len) && read_str(val, len); } - - // Read a 4-byte type code (no endian swap), and if that succeeds (beware // of EOF or other errors), then also read a 32 bit size (subject to // endian swap). @@ -99,7 +97,20 @@ class IffInput final : public ImageInput { bool read_chunk(uint8_t type[4], uint32_t& size) { - return read_type(type) && read(&size); + bool ok = read_type(type) && read(&size); + if (!ok) { + errorfmt("IFF error io could not read chunk"); + return false; + } + // Check that the chunk size as it represents itself actually fits + // within the remainder of the file. If it does not, clearly something + // is corrupted. + if (size_t(iotell()) + size > ioproxy()->size()) { + errorfmt("nonsensical chunk size {} at position {} file size {}\n", + size, iotell(), ioproxy()->size()); + return false; + } + return true; } }; @@ -297,7 +308,7 @@ IffInput::read_header() // chunk type: FOR4 if (std::memcmp(chunktype, iff_for4_tag, 4) == 0) { if (!ioread(&chunktype, 1, sizeof(chunktype))) { - errorfmt("IFF error io seek failed for type"); + errorfmt("IFF error io read failed for type"); return false; } @@ -306,7 +317,6 @@ IffInput::read_header() for (;;) { if (!read_chunk(chunktype, size)) return false; - chunksize = align_chunk(size, 4); // chunk type: TBHD @@ -355,7 +365,11 @@ IffInput::read_header() // RGB(A) format if (flags & RGBA) { // test if black is set - OIIO_DASSERT(!(flags & BLACK)); + if (flags & BLACK) { + errorfmt("Invalid header flag combination {:x}", + flags); + return false; + } if (flags & RGB) m_header.rgba_count = 3; @@ -379,14 +393,26 @@ IffInput::read_header() OIIO_DASSERT(bytes == 0); } + // Validate the color channel configuration. A + // legitimate Maya IFF stores RGB (3) or RGBA (4) + // color channels (optionally plus Z). Corrupt flag + // combinations -- such as ALPHA without RGB -- yield + // channel counts the format never produces and that + // the tile decoder cannot represent, which previously + // led to out-of-bounds writes while decompressing. + if ((flags & RGBA) && m_header.rgba_count != 3 + && m_header.rgba_count != 4) { + errorfmt( + "IFF error unsupported channel configuration (flags {:#x})", + flags); + return false; + } + // read chunks for (;;) { // get type - if (!read_chunk(chunktype, size)) { - errorfmt("IFF error read type size failed"); + if (!read_chunk(chunktype, size)) return false; - } - chunksize = align_chunk(size, 4); // chunk type: AUTH @@ -577,10 +603,8 @@ IffInput::readimg() while ((rgbatiles < m_header.tiles && m_header.rgba_count > 0) || (ztiles < m_header.tiles && m_header.zbuffer > 0)) { // get type and length - if (!read_chunk(chunktype, size)) { - errorfmt("IFF error io could not read rgb(a) type"); + if (!read_chunk(chunktype, size)) return false; - } chunksize = align_chunk(size, 4); // chunk type: RGBA @@ -598,6 +622,11 @@ IffInput::readimg() // get image size // skip coordinates, uint16_t (2) * 4 = 8 + if (chunksize <= 8) { + errorfmt("nonsensical RGBA chunk size {} (must be > 8)\n", + size); + return false; + } uint32_t image_size = chunksize - 8; // check tile diff --git a/testsuite/iff/ref/out.txt b/testsuite/iff/ref/out.txt index e0720171a8..3d1a98839a 100644 --- a/testsuite/iff/ref/out.txt +++ b/testsuite/iff/ref/out.txt @@ -7,3 +7,11 @@ src/tiny_rgba16z.iff : 1 x 1, 5 channel, uint16/uint16/uint16/uint16/float SHA-1: 1B90DDB531A3ABD137C5050BA7A955AD4F711B4C channel list: R (uint16), G (uint16), B (uint16), A (uint16), Z (float) tile size: 1 x 1 +Reading src/bad_rgba_chunk_size.iff +oiiotool ERROR: -info : SHA-1: nonsensical RGBA chunk size 0 (must be > 8) +Full command line was: +> oiiotool --info -v -a --hash src/bad_rgba_chunk_size.iff +src/bad_rgba_chunk_size.iff : 64 x 64, 3 channel, uint16 iff + channel list: R, G, B + tile size: 1 x 1 + compression: "rle" diff --git a/testsuite/iff/run.py b/testsuite/iff/run.py index 41ceabe185..24ba1e1308 100755 --- a/testsuite/iff/run.py +++ b/testsuite/iff/run.py @@ -4,6 +4,8 @@ # SPDX-License-Identifier: Apache-2.0 # https://github.com/AcademySoftwareFoundation/OpenImageIO +redirect = ' >> out.txt 2>&1 ' + command += oiiotool (OIIO_TESTSUITE_IMAGEDIR+"/grid.tif --scanline -o gridscanline.iff") command += diff_command (OIIO_TESTSUITE_IMAGEDIR+"/grid.tif", "gridscanline.iff") command += oiiotool (OIIO_TESTSUITE_IMAGEDIR+"/grid.tif --tile 64 64 -o gridtile.iff") @@ -11,4 +13,6 @@ # Regression test: verify reading of 16 bit rgba + float z (used to have a # buffer overrun) -command += info_command("src//tiny_rgba16z.iff", hash=True) +command += info_command("src/tiny_rgba16z.iff", hash=True) +# Regression test: chunk size 0 caused subtraction underflow +command += info_command("src/bad_rgba_chunk_size.iff", hash=True, failureok=True) diff --git a/testsuite/iff/src/bad_rgba_chunk_size.iff b/testsuite/iff/src/bad_rgba_chunk_size.iff new file mode 100644 index 0000000000000000000000000000000000000000..eb6a8c491bbc53a7b062967494e5f6cafcffde39 GIT binary patch literal 646 zcmZ?s4>Dn3U_9aM>FXZi Date: Tue, 30 Jun 2026 18:32:23 -0400 Subject: [PATCH 2/2] fix(iff): reject ZBUFFER-only headers to prevent tile buffer overflow A crafted Maya IFF with the TBHD ZBUFFER flag set but RGBA clear took the ZBUFFER-only branch, which set rgba_count=1/rgba_bits=32 while leaving zbuffer=0. open() then exposed the image as 1-channel UINT16 (so callers size their tile buffer from ImageSpec::tile_bytes(true)), but the internal pixel_bytes() stayed 32-bit. read_native_tile() copied by the internal pixel size, writing past the smaller caller buffer -- a heap out-of-bounds write. Such files can never be decoded anyway: readimg()'s tile loop only handles 8- and 16-bit RGBA pixels and explicitly errors on 32-bit. So reject ZBUFFER-only headers at open() instead of fabricating an inconsistent spec. Also: - Generalize the channel-config check to require rgba_count in {3,4}, which additionally rejects headers with no color flags at all. - Clear m_buf when readimg() fails. readimg() resizes m_buf before decoding, so a partial/failed decode left a non-empty buffer that a later tile request would reuse, skipping the (failed) decode and copying stale data. Adds regression test src/zbuffer_only.iff. Assisted-by: Claude Code / Claude Opus 4.8 Signed-off-by: Larry Gritz --- src/iff.imageio/iffinput.cpp | 33 +++++++++++++++++++---------- testsuite/iff/ref/out.txt | 4 ++++ testsuite/iff/run.py | 4 ++++ testsuite/iff/src/zbuffer_only.iff | Bin 0 -> 88 bytes 4 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 testsuite/iff/src/zbuffer_only.iff diff --git a/src/iff.imageio/iffinput.cpp b/src/iff.imageio/iffinput.cpp index 50ed29e4d5..fd18d33023 100644 --- a/src/iff.imageio/iffinput.cpp +++ b/src/iff.imageio/iffinput.cpp @@ -386,21 +386,27 @@ IffInput::read_header() } // Z format. else if (flags & ZBUFFER) { - // todo: we have not seen a sample of this - m_header.rgba_count = 1; - m_header.rgba_bits = 32; // 32bit - // NOTE: Z_F32 support - not supported - OIIO_DASSERT(bytes == 0); + // A ZBUFFER-only (no RGBA) image would be 32-bit + // depth data. The tile decoder only handles 8- and + // 16-bit RGBA pixels, so such a file can never be + // decoded. Exposing it as a 16-bit ImageSpec while + // the internal pixel size stayed 32-bit previously + // caused read_native_tile to write past the + // caller's tile buffer. Reject it outright. + errorfmt( + "IFF error ZBUFFER-only (Z-depth) images are not supported"); + return false; } // Validate the color channel configuration. A // legitimate Maya IFF stores RGB (3) or RGBA (4) // color channels (optionally plus Z). Corrupt flag - // combinations -- such as ALPHA without RGB -- yield - // channel counts the format never produces and that - // the tile decoder cannot represent, which previously - // led to out-of-bounds writes while decompressing. - if ((flags & RGBA) && m_header.rgba_count != 3 + // combinations -- such as ALPHA without RGB, or no + // color flags at all -- yield channel counts the + // format never produces and that the tile decoder + // cannot represent, which previously led to + // out-of-bounds writes while decompressing. + if (m_header.rgba_count != 3 && m_header.rgba_count != 4) { errorfmt( "IFF error unsupported channel configuration (flags {:#x})", @@ -538,8 +544,13 @@ IffInput::read_native_tile(int subimage, int miplevel, int x, int y, int /*z*/, lock_guard lock(*this); if (m_buf.empty()) { - if (!readimg()) + if (!readimg()) { + // readimg() may have partially resized m_buf before failing. + // Clear it so a subsequent tile request does not skip the + // (failed) decode and copy from a half-initialized buffer. + m_buf.clear(); return false; + } } } diff --git a/testsuite/iff/ref/out.txt b/testsuite/iff/ref/out.txt index 3d1a98839a..25960a1142 100644 --- a/testsuite/iff/ref/out.txt +++ b/testsuite/iff/ref/out.txt @@ -15,3 +15,7 @@ src/bad_rgba_chunk_size.iff : 64 x 64, 3 channel, uint16 iff channel list: R, G, B tile size: 1 x 1 compression: "rle" +oiiotool ERROR: read : "src/zbuffer_only.iff": IFF error ZBUFFER-only (Z-depth) images are not supported +IFF error could not read header +Full command line was: +> oiiotool --info -v -a --hash src/zbuffer_only.iff diff --git a/testsuite/iff/run.py b/testsuite/iff/run.py index 24ba1e1308..9a0c940b46 100755 --- a/testsuite/iff/run.py +++ b/testsuite/iff/run.py @@ -16,3 +16,7 @@ command += info_command("src/tiny_rgba16z.iff", hash=True) # Regression test: chunk size 0 caused subtraction underflow command += info_command("src/bad_rgba_chunk_size.iff", hash=True, failureok=True) +# Regression test: a ZBUFFER-only (no RGBA) header advertised a 16-bit +# ImageSpec while the internal pixel size stayed 32-bit, causing +# read_native_tile to write past the caller's tile buffer. Must be rejected. +command += info_command("src/zbuffer_only.iff", hash=True, failureok=True) diff --git a/testsuite/iff/src/zbuffer_only.iff b/testsuite/iff/src/zbuffer_only.iff new file mode 100644 index 0000000000000000000000000000000000000000..130fae914355171255f582d1622f50ffd1b2e5ee GIT binary patch literal 88 zcmZ?s4>Dn3U