From 8f5641c5a9fb98c8d02e93d619b3d301fc14cec5 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sun, 26 Jul 2026 15:12:43 -0700 Subject: [PATCH] fix(rla): terminate the subimage chain and bounds-check the offset table RLA subimages are concatenated, each header's NextOffset giving the start of the next. Nothing required NextOffset to advance, so a file pointing it at itself or backwards supplied subimages endlessly and any caller enumerating them never terminated. Require it to point past the current header. seek_subimage() now also gives up its current subimage before overwriting the header and scanline offset table, so failing partway leaves no current subimage rather than a mismatched header, table, and spec. read_native_scanline() bounds-checks the scanline index against the offset table instead of asserting that the table matches the spec height -- false for field-rendered images, where the height is halved but the table is not. Assisted-by: Claude Code / Claude Opus 5 Signed-off-by: Larry Gritz --- src/rla.imageio/rlainput.cpp | 54 ++++++--- testsuite/rla/ref/out.txt | 29 +++++ testsuite/rla/run.py | 11 ++ testsuite/rla/src/bomb.rla | Bin 0 -> 1204 bytes testsuite/rla/src/field-rendered.rla | Bin 0 -> 804 bytes testsuite/rla/src/make_malformed_rla.py | 149 ++++++++++++++++++++++++ testsuite/rla/src/subimage-loop.rla | Bin 0 -> 1608 bytes 7 files changed, 229 insertions(+), 14 deletions(-) create mode 100644 testsuite/rla/src/bomb.rla create mode 100644 testsuite/rla/src/field-rendered.rla create mode 100644 testsuite/rla/src/make_malformed_rla.py create mode 100644 testsuite/rla/src/subimage-loop.rla diff --git a/src/rla.imageio/rlainput.cpp b/src/rla.imageio/rlainput.cpp index f71234c34e..d936cf0fae 100644 --- a/src/rla.imageio/rlainput.cpp +++ b/src/rla.imageio/rlainput.cpp @@ -48,6 +48,7 @@ class RLAInput final : public ImageInput { int m_subimage; ///< Current subimage index std::vector m_sot; ///< Scanline offsets table int m_stride; ///< Number of bytes a contig pixel takes + int64_t m_header_offset; ///< File offset of the current header /// Reset everything to initial state /// @@ -55,6 +56,9 @@ class RLAInput final : public ImageInput { { ioproxy_clear(); m_buf.clear(); + m_sot.clear(); + m_subimage = -1; + m_header_offset = -1; } /// Helper: read buf[0..nitems-1], swap endianness if necessary @@ -165,8 +169,8 @@ RLAInput::open(const std::string& name, ImageSpec& newspec) return false; ioseek(0); - // set a bogus subimage index so that seek_subimage actually seeks - m_subimage = 1; + // no current subimage yet, so seek_subimage actually seeks + m_subimage = -1; bool ok = seek_subimage(0, 0); if (!ok) { @@ -185,6 +189,7 @@ RLAInput::read_header() // Read the image header, which should have the same exact layout as // the m_rla structure (except for endianness issues). static_assert(sizeof(m_rla) == 740, "Bad RLA struct size"); + const int64_t header_offset = iotell(); if (!read(&m_rla)) { errorfmt("RLA could not read the image header"); return false; @@ -225,6 +230,7 @@ RLAInput::read_header() errorfmt("RLA could not read the scanline offset table"); return false; } + m_header_offset = header_offset; return true; } @@ -236,34 +242,48 @@ RLAInput::seek_subimage(int subimage, int miplevel) if (miplevel != 0 || subimage < 0) return false; - if (subimage == current_subimage()) + if (subimage == m_subimage) return true; // already on the right level // RLA images allow multiple subimages; they are simply concatenated // together, with image N's header field NextOffset giving the // absolute offset of the start of image N+1. - int diff = subimage - current_subimage(); - if (subimage - current_subimage() < 0) { - // If we are requesting an image earlier than the current one, - // reset to the first subimage. + + // We're about to overwrite the header and the scanline offset table, so + // give up the current subimage first -- failing partway then leaves us + // with none, rather than a mismatched header, table, and spec. + int cur = m_subimage; + m_subimage = -1; + if (cur < 0 || subimage < cur) { + // No current subimage, or we want one earlier than the current: + // start over from the first subimage. ioseek(0); if (!read_header()) return false; // read_header always calls error() - diff = subimage; - m_subimage = 0; + cur = 0; } // forward scrolling -- skip subimages until we're at the right place - while (diff > 0 && m_subimage < subimage && m_rla.NextOffset != 0) { + while (cur < subimage && m_rla.NextOffset != 0) { + // Subimages are concatenated, so each NextOffset must point past the + // current header. Without this, a file whose NextOffset points at + // itself or backwards supplies subimages endlessly and any caller + // enumerating them never terminates. + int64_t min_next = m_header_offset + int64_t(sizeof(m_rla)); + if (int64_t(m_rla.NextOffset) < min_next) { + errorfmt( + "Subimage offset {} does not advance past the header at {}. Corrupted file?", + m_rla.NextOffset, m_header_offset); + return false; + } if (!ioseek(m_rla.NextOffset)) { errorfmt("Could not seek to header offset. Corrupted file?"); return false; } if (!read_header()) return false; // read_header always calls error() - --diff; - ++m_subimage; + ++cur; } - if (diff > 0 && m_rla.NextOffset == 0) { // no more subimages to read + if (cur < subimage) { // no more subimages to read errorfmt("Unknown subimage"); return false; } @@ -699,7 +719,13 @@ RLAInput::read_native_scanline(int subimage, int miplevel, int y, int /*z*/, // Invalid scanline return false; } - OIIO_DASSERT(m_sot.size() == size_t(m_spec.height)); + // The table has one entry per scanline of the active window, but the + // spec height is halved for field-rendered images, so the two sizes + // legitimately differ -- bounds check rather than assume they match. + if (size_t(y) >= m_sot.size()) { + errorfmt("Scanline {} has no offset table entry. Corrupted file?", y); + return false; + } if (!ioseek(m_sot[y])) return false; diff --git a/testsuite/rla/ref/out.txt b/testsuite/rla/ref/out.txt index 8ec7ddff8c..d03f5d9fd2 100644 --- a/testsuite/rla/ref/out.txt +++ b/testsuite/rla/ref/out.txt @@ -331,5 +331,34 @@ Full command line was: oiiotool ERROR: read : "src/crash-badrle.rla": Read error: malformed RLE record Full command line was: > oiiotool src/crash-badrle.rla -o crash8.exr +oiiotool ERROR: read : "src/bomb.rla": rla header claims a 6549 MB image from a 1204 byte file; probably a corrupt or malicious header +Full command line was: +> oiiotool --info -a -v src/bomb.rla +Reading src/subimage-loop.rla +src/subimage-loop.rla : 8 x 8, 1 channel, uint8 rla + 2 subimages: 8x8 [u8], 8x8 [u8] + subimage 0: 8 x 8, 1 channel, uint8 rla + SHA-1: CE332FD2CCAECF316970B3C1AF6BADB409257222 + channel list: Y + compression: "rle" + oiio:BitsPerSample: 8 + oiio:ColorSpace: "lin_rec709_scene" + subimage 1: 8 x 8, 1 channel, uint8 rla + SHA-1: CE332FD2CCAECF316970B3C1AF6BADB409257222 + channel list: Y + compression: "rle" + oiio:BitsPerSample: 8 + oiio:ColorSpace: "lin_rec709_scene" +Reading src/field-rendered.rla +src/field-rendered.rla : 8 x 4, 1 channel, uint8 rla + SHA-1: E8F26115CED9DD22BAD3704E3073E56528CCE721 + channel list: Y + pixel data origin: x=0, y=-4 + full/display size: 8 x 8 + full/display origin: 0, 0 + compression: "rle" + oiio:BitsPerSample: 8 + oiio:ColorSpace: "lin_rec709_scene" + rla:FieldRendered: 1 Comparing "rlacrop.rla" and "ref/rlacrop.rla" PASS diff --git a/testsuite/rla/run.py b/testsuite/rla/run.py index 172efdca7d..8f51dd38e3 100755 --- a/testsuite/rla/run.py +++ b/testsuite/rla/run.py @@ -27,4 +27,15 @@ command += oiiotool("src/crash-5159.rla -o crash7.exr", failureok = True) command += oiiotool("src/crash-badrle.rla -o crash8.exr", failureok = True) +# Malformed inputs built by src/make_malformed_rla.py. +# A 1.2 KB file claiming a 6.8 GB image must be rejected before anything is +# sized from the spec. +command += oiiotool("--info -a -v src/bomb.rla", failureok = True) +# A subimage whose NextOffset points at itself must not present an endless +# supply of subimages; enumeration has to terminate. +command += oiiotool("--info -a -v --hash src/subimage-loop.rla", failureok = True) +# Field-rendered images halve the height, so the scanline offset table has +# more entries than there are scanlines. Reading must stay in bounds. +command += oiiotool("--info -v --hash src/field-rendered.rla", failureok = True) + outputs = [ "rlacrop.rla", 'out.txt' ] diff --git a/testsuite/rla/src/bomb.rla b/testsuite/rla/src/bomb.rla new file mode 100644 index 0000000000000000000000000000000000000000..d5152adfe13c68c25f4d0a65fdaaccb674626ba8 GIT binary patch literal 1204 zcmeIwy-meH3;}Olo2vQHXtKh&ns?(8{tOC2pJ(GWQ5cYJ_$AeQDNzm z?aGqvbG%<7;_dvM)z@)~(^Y(2UY}pI|M9jfItsog?Bd(a|$7GBJ}WR2mB_8#_nDeWIgh wU}Rz@Q>Zi+RyKByh=)W+&%nsUOr}t2EUawo91)L+j-G*$iJ45H(tdi@PeO7#EC2ui literal 0 HcmV?d00001 diff --git a/testsuite/rla/src/field-rendered.rla b/testsuite/rla/src/field-rendered.rla new file mode 100644 index 0000000000000000000000000000000000000000..bf6efae56ae788856cdd59b0b63faeae50eeb5d3 GIT binary patch literal 804 zcmZQzU|?rJ#S9FLKoN%j{|xmEK#Wl^8UmDs060AnZ3G8UB@7d-3BN*S76t}ppqb1( XKr8^nB0www#4h', n) + + def l(n): + return struct.pack('>i', n) + + def c(n, txt=b''): + return txt.ljust(n, b'\0') + + h = b'' + h += s(0) + s(active_w - 1) # WindowLeft, WindowRight + h += s(0) + s(active_h - 1) # WindowBottom, WindowTop + h += s(0) + s(active_w - 1) # ActiveLeft, ActiveRight + h += s(0) + s(active_h - 1) # ActiveBottom, ActiveTop + h += s(0) # FrameNumber + h += s(chan_type) # ColorChannelType + h += s(nchan) + s(nmatte) + s(naux) # channel counts + h += s(struct.unpack('>h', struct.pack('>H', revision))[0]) + h += c(16, b'1.0') # Gamma + h += c(24) + c(24) + c(24) + c(24) # Red/Green/Blue chroma, WhitePoint + h += l(0) # JobNumber + h += c(128) + c(128) # FileName, Description + h += c(64) + c(32) + c(32) # ProgramName, MachineName, UserName + h += c(20) # DateCreated + h += c(24) + c(8) # Aspect, AspectRatio + h += c(32) # ColorChannel + h += s(field_rendered) + h += c(12) + c(32) # Time, Filter + h += s(chan_bits) + h += s(matte_type) + s(matte_bits) + h += s(aux_type) + s(aux_bits) + h += c(32) + c(36) # AuxData, Reserved + h += l(next_offset) # NextOffset + assert len(h) == 740, len(h) + return h + + +def scanline(width, nchan_total, chan_bytes=1): + """One scanline record: per channel a uint16 length plus its RLE payload.""" + out = b'' + for _ in range(nchan_total): + for _ in range(chan_bytes): + payload = b'' + left = width + while left > 0: + n = min(left, 128) + payload += bytes([n - 1, 0x40]) # n copies of 0x40 + left -= n + out += struct.pack('>H', len(payload)) + payload + return out + + +def build(subimages): + """Concatenate subimages, resolving NextOffset and scanline offsets.""" + blobs = [] + for si in subimages: + body = b'' + sot = [] + base = 740 + 4 * si['h'] + for _ in range(si['h']): + sot.append(base + len(body)) + body += scanline(si['w'], si.get('nchan', 1)) + blobs.append((si, sot, body)) + + offsets, pos = [], 0 + for si, sot, body in blobs: + offsets.append(pos) + pos += 740 + 4 * len(sot) + len(body) + + out = b'' + for i, (si, sot, body) in enumerate(blobs): + nxt = offsets[i + 1] if i + 1 < len(blobs) else 0 + kw = {k: v for k, v in si.items() if k not in ('w', 'h', 'nchan')} + hdr = header(si['w'], si['h'], nchan=si.get('nchan', 1), + next_offset=nxt, **kw) + adj = [o + offsets[i] for o in sot] + out += hdr + b''.join(struct.pack('>I', o) for o in adj) + body + return out + + +def main(outdir): + def write(name, data): + with open(outdir + '/' + name, 'wb') as f: + f.write(data) + print('wrote %s (%d bytes)' % (name, len(data))) + + # 1.2 KB claiming 65535 x 100 x 262 channels of uint32 = 6.8 GB. + # Only the 400-byte offset table scales with the declared size, so the + # ratio of declared to actual bytes is about 5.7 million. + h = bytearray(header(8, 100, nchan=3, nmatte=3, naux=256, + chan_type=CT_DWORD, chan_bits=32, + matte_type=CT_DWORD, matte_bits=32, + aux_type=CT_DWORD, aux_bits=32)) + struct.pack_into('>hh', h, OFF_WINDOW_LEFT, -32768, 32766) + struct.pack_into('>hh', h, OFF_ACTIVE_LEFT, -32768, 32766) + sot = b''.join(struct.pack('>I', 740 + 400 + i * 16) for i in range(100)) + write('bomb.rla', bytes(h) + sot + b'\0' * 64) + + # Subimage 1's NextOffset points at itself, so walking the subimage + # chain never terminates and the file appears to hold infinitely many + # subimages. + off1 = 740 + 4 * 8 + len(scanline(8, 1)) * 8 + d = bytearray(build([{'w': 8, 'h': 8}, {'w': 8, 'h': 8}])) + struct.pack_into('>i', d, off1 + OFF_NEXT_OFFSET, off1) + write('subimage-loop.rla', bytes(d)) + + # Field-rendered: the offset table has one entry per scanline of the + # active window but the height is halved, so table size and height + # disagree by design. Must read cleanly rather than trip an assertion. + write('field-rendered.rla', build([{'w': 8, 'h': 8, 'field_rendered': 1}])) + + +if __name__ == '__main__': + main(sys.argv[1] if len(sys.argv) > 1 else '.') diff --git a/testsuite/rla/src/subimage-loop.rla b/testsuite/rla/src/subimage-loop.rla new file mode 100644 index 0000000000000000000000000000000000000000..5f42124706350a2a5c6f233e762db2a5d090cbdb GIT binary patch literal 1608 zcmZQzU|?rJ#S9FLKoN%j{|xmEK#Wl^8UpkT0S=&LFiZt|nN=7Vm|1`rXeKic5DNgY z2oOsEu?!F^Ffg$@;Kiu<1C(Dz^9U7UX4D9>LjaaPKr|HqD37pd0I?1b0}FjN6Ckz# QVjCcK0Ad%s`2(1L0CBqv=>Px# literal 0 HcmV?d00001