fix(rla): terminate the subimage chain and bounds-check the offset table

src/rla.imageio/rlainput.cpp

@@ -48,13 +48,17 @@ class RLAInput final : public ImageInput {
     int m_subimage;                    ///< Current subimage index
     std::vector<uint32_t> 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
     ///
     void init()
     {
         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;
 

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

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' ]

testsuite/rla/src/make_malformed_rla.py

@@ -0,0 +1,149 @@
+#!/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 .rla fixtures in this directory.
+
+The files it writes are committed, so this only needs to be run if they must
+be regenerated:
+
+    python3 make_malformed_rla.py .
+
+RLA is big-endian throughout: a 740-byte header, then one uint32 per scanline
+giving that scanline's absolute file offset, then the RLE records. Because the
+offset table costs only 4 bytes per scanline, a tiny file can describe an
+enormous image -- which is what bomb.rla exercises.
+"""
+
+import struct
+import sys
+
+CT_BYTE, CT_WORD, CT_DWORD, CT_FLOAT = 0, 1, 2, 4
+
+# Byte offsets of the fields we patch, within the 740-byte header.
+OFF_WINDOW_LEFT = 0
+OFF_ACTIVE_LEFT = 8
+OFF_NEXT_OFFSET = 736
+
+
+def header(active_w, active_h, nchan=1, nmatte=0, naux=0,
+           chan_type=CT_BYTE, chan_bits=8,
+           matte_type=CT_BYTE, matte_bits=8,
+           aux_type=CT_BYTE, aux_bits=8,
+           next_offset=0, field_rendered=0, revision=0xFFFE):
+    def s(n):
+        return struct.pack('>h', 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 '.')