fix(rla): Harden against corrupted files (AcademySoftwareFoundation#5153)

1. Harden against buffer overruns from malformed RLE blocks. The simple
fix is the `if (count > ...)` clauses on lines 503 & 510, but I also
took the opportunity to rewrite decode_rle_span using spans rather than
raw pointers.

Fixes AcademySoftwareFoundation#5152

2. Clean up seeking methodology to be more robust.

Fixes AcademySoftwareFoundation#5159

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Author: lgritz

src/rla.imageio/rlainput.cpp

@@ -86,11 +86,11 @@ class RLAInput final : public ImageInput {
     bool decode_channel_group(int first_channel, short num_channels,
                               short num_bits, int y);
 
-    /// Helper: decode a span of n RLE-encoded bytes from encoded[0..elen-1]
+    /// Helper: decode a span of n RLE-encoded bytes from encoded[]
     /// into buf[0],buf[stride],buf[2*stride]...buf[(n-1)*stride].
     /// Return the number of encoded bytes we ate to fill buf.
-    size_t decode_rle_span(unsigned char* buf, int n, int stride,
-                           const char* encoded, size_t elen);
+    size_t decode_rle_span(span<unsigned char> buf, int n, int stride,
+                           cspan<char> encoded);
 
     /// Helper: determine channel TypeDesc
     inline TypeDesc get_channel_typedesc(short chan_type, short chan_bits);
@@ -245,17 +245,19 @@ RLAInput::seek_subimage(int subimage, int miplevel)
         ioseek(0);
         if (!read_header())
             return false;  // read_header always calls error()
-        diff = subimage;
+        diff       = subimage;
+        m_subimage = 0;
     }
     // forward scrolling -- skip subimages until we're at the right place
-    while (diff > 0 && m_rla.NextOffset != 0) {
+    while (diff > 0 && m_subimage < subimage && m_rla.NextOffset != 0) {
         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;
     }
     if (diff > 0 && m_rla.NextOffset == 0) {  // no more subimages to read
         errorfmt("Unknown subimage");
@@ -495,23 +497,28 @@ RLAInput::close()
 
 
 size_t
-RLAInput::decode_rle_span(unsigned char* buf, int n, int stride,
-                          const char* encoded, size_t elen)
+RLAInput::decode_rle_span(span<unsigned char> buf, int n, int stride,
+                          cspan<char> encoded)
 {
-    size_t e = 0;
+    size_t e    = 0;               // position we're reading in encoded
+    size_t elen = encoded.size();  // Number of encoded bytes to decode
+    size_t b    = 0;               // postition we're writing in buf
     while (n > 0 && e < elen) {
         int count = (signed char)encoded[e++];
         if (count >= 0) {
             // run count positive: value repeated count+1 times
-            for (int i = 0; i <= count && n && e < elen;
-                 ++i, buf += stride, --n)
-                *buf = encoded[e];
+            if (count + 1 > n)
+                break;  // asking for a count that will overrun the buffer
+            for (int i = 0; i <= count; ++i, b += stride, --n)
+                buf[b] = encoded[e];
             ++e;
         } else {
             // run count negative: repeat bytes literally
+            if (count > n)
+                break;       // asking for a count that will overrun the buffer
             count = -count;  // make it positive
-            for (; count && n > 0 && e < elen; --count, buf += stride, --n)
-                *buf = encoded[e++];
+            for (; count && n > 0 && e < elen; --count, b += stride, --n)
+                buf[b] = encoded[e++];
         }
     }
     if (n != 0) {
@@ -545,7 +552,7 @@ RLAInput::decode_channel_group(int first_channel, short num_channels,
         offset    = 0;
         pixelsize = m_spec.pixel_bytes(true);
         for (int i = 0; i < first_channel; ++i)
-            offset += m_spec.channelformats[i].size();
+            offset += m_spec.channelformat(i).size();
     }
 
     // Read the big-endian values into the buffer.
@@ -588,9 +595,10 @@ RLAInput::decode_channel_group(int first_channel, short num_channels,
         // and strides to decode_rle_span.
         size_t eoffset = 0;
         for (int bytes = 0; bytes < chsize && length > 0; ++bytes) {
-            size_t e = decode_rle_span(&m_buf[offset + c * chsize + bytes],
+            size_t e = decode_rle_span(make_span(m_buf).subspan(
+                                           offset + c * chsize + bytes),
                                        m_spec.width, pixelsize,
-                                       &encoded[eoffset], length);
+                                       make_span(encoded).subspan(eoffset));
             if (!e)
                 return false;
             eoffset += e;

testsuite/rla/ref/out.txt

@@ -322,5 +322,11 @@ Full command line was:
 oiiotool ERROR: read : "src/crash-1.rla": rla image full/display resolution must be at least 1x1, but the file specified -281x18999x1. Possible corrupt input?
 Full command line was:
 > oiiotool src/crash-1.rla -o crash5.exr
+oiiotool ERROR: read : "src/crash-5152.rla": Read error: malformed RLE record
+Full command line was:
+> oiiotool src/crash-5152.rla -o crash6.exr
+oiiotool ERROR: read : "src/crash-5159.rla": Read error: couldn't read RLE data span
+Full command line was:
+> oiiotool src/crash-5159.rla -o crash7.exr
 Comparing "rlacrop.rla" and "ref/rlacrop.rla"
 PASS

testsuite/rla/run.py

@@ -23,5 +23,7 @@
 command += oiiotool("src/crash-1629.rla -o crash3.exr", failureok = True)
 command += oiiotool("src/crash-3951.rla -o crash4.exr", failureok = True)
 command += oiiotool("src/crash-1.rla -o crash5.exr", failureok = True)
+command += oiiotool("src/crash-5152.rla -o crash6.exr", failureok = True)
+command += oiiotool("src/crash-5159.rla -o crash7.exr", failureok = True)
 
 outputs = [ "rlacrop.rla", 'out.txt' ]