Skip to content

Commit 247e61b

Browse files
authored
fix(rla): Harden against corrupted files (#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 #5152 2. Clean up seeking methodology to be more robust. Fixes #5159 --------- Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 4001154 commit 247e61b

5 files changed

Lines changed: 32 additions & 16 deletions

File tree

src/rla.imageio/rlainput.cpp

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@ class RLAInput final : public ImageInput {
8686
bool decode_channel_group(int first_channel, short num_channels,
8787
short num_bits, int y);
8888

89-
/// Helper: decode a span of n RLE-encoded bytes from encoded[0..elen-1]
89+
/// Helper: decode a span of n RLE-encoded bytes from encoded[]
9090
/// into buf[0],buf[stride],buf[2*stride]...buf[(n-1)*stride].
9191
/// Return the number of encoded bytes we ate to fill buf.
92-
size_t decode_rle_span(unsigned char* buf, int n, int stride,
93-
const char* encoded, size_t elen);
92+
size_t decode_rle_span(span<unsigned char> buf, int n, int stride,
93+
cspan<char> encoded);
9494

9595
/// Helper: determine channel TypeDesc
9696
inline TypeDesc get_channel_typedesc(short chan_type, short chan_bits);
@@ -245,17 +245,19 @@ RLAInput::seek_subimage(int subimage, int miplevel)
245245
ioseek(0);
246246
if (!read_header())
247247
return false; // read_header always calls error()
248-
diff = subimage;
248+
diff = subimage;
249+
m_subimage = 0;
249250
}
250251
// forward scrolling -- skip subimages until we're at the right place
251-
while (diff > 0 && m_rla.NextOffset != 0) {
252+
while (diff > 0 && m_subimage < subimage && m_rla.NextOffset != 0) {
252253
if (!ioseek(m_rla.NextOffset)) {
253254
errorfmt("Could not seek to header offset. Corrupted file?");
254255
return false;
255256
}
256257
if (!read_header())
257258
return false; // read_header always calls error()
258259
--diff;
260+
++m_subimage;
259261
}
260262
if (diff > 0 && m_rla.NextOffset == 0) { // no more subimages to read
261263
errorfmt("Unknown subimage");
@@ -490,23 +492,28 @@ RLAInput::close()
490492

491493

492494
size_t
493-
RLAInput::decode_rle_span(unsigned char* buf, int n, int stride,
494-
const char* encoded, size_t elen)
495+
RLAInput::decode_rle_span(span<unsigned char> buf, int n, int stride,
496+
cspan<char> encoded)
495497
{
496-
size_t e = 0;
498+
size_t e = 0; // position we're reading in encoded
499+
size_t elen = encoded.size(); // Number of encoded bytes to decode
500+
size_t b = 0; // postition we're writing in buf
497501
while (n > 0 && e < elen) {
498502
int count = (signed char)encoded[e++];
499503
if (count >= 0) {
500504
// run count positive: value repeated count+1 times
501-
for (int i = 0; i <= count && n && e < elen;
502-
++i, buf += stride, --n)
503-
*buf = encoded[e];
505+
if (count + 1 > n)
506+
break; // asking for a count that will overrun the buffer
507+
for (int i = 0; i <= count; ++i, b += stride, --n)
508+
buf[b] = encoded[e];
504509
++e;
505510
} else {
506511
// run count negative: repeat bytes literally
512+
if (count > n)
513+
break; // asking for a count that will overrun the buffer
507514
count = -count; // make it positive
508-
for (; count && n > 0 && e < elen; --count, buf += stride, --n)
509-
*buf = encoded[e++];
515+
for (; count && n > 0 && e < elen; --count, b += stride, --n)
516+
buf[b] = encoded[e++];
510517
}
511518
}
512519
if (n != 0) {
@@ -540,7 +547,7 @@ RLAInput::decode_channel_group(int first_channel, short num_channels,
540547
offset = 0;
541548
pixelsize = m_spec.pixel_bytes(true);
542549
for (int i = 0; i < first_channel; ++i)
543-
offset += m_spec.channelformats[i].size();
550+
offset += m_spec.channelformat(i).size();
544551
}
545552

546553
// Read the big-endian values into the buffer.
@@ -583,9 +590,10 @@ RLAInput::decode_channel_group(int first_channel, short num_channels,
583590
// and strides to decode_rle_span.
584591
size_t eoffset = 0;
585592
for (int bytes = 0; bytes < chsize && length > 0; ++bytes) {
586-
size_t e = decode_rle_span(&m_buf[offset + c * chsize + bytes],
593+
size_t e = decode_rle_span(make_span(m_buf).subspan(
594+
offset + c * chsize + bytes),
587595
m_spec.width, pixelsize,
588-
&encoded[eoffset], length);
596+
make_span(encoded).subspan(eoffset));
589597
if (!e)
590598
return false;
591599
eoffset += e;

testsuite/rla/ref/out.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,5 +322,11 @@ Full command line was:
322322
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?
323323
Full command line was:
324324
> oiiotool src/crash-1.rla -o crash5.exr
325+
oiiotool ERROR: read : "src/crash-5152.rla": Read error: malformed RLE record
326+
Full command line was:
327+
> oiiotool src/crash-5152.rla -o crash6.exr
328+
oiiotool ERROR: read : "src/crash-5159.rla": Read error: couldn't read RLE data span
329+
Full command line was:
330+
> oiiotool src/crash-5159.rla -o crash7.exr
325331
Comparing "rlacrop.rla" and "ref/rlacrop.rla"
326332
PASS

testsuite/rla/run.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,7 @@
2323
command += oiiotool("src/crash-1629.rla -o crash3.exr", failureok = True)
2424
command += oiiotool("src/crash-3951.rla -o crash4.exr", failureok = True)
2525
command += oiiotool("src/crash-1.rla -o crash5.exr", failureok = True)
26+
command += oiiotool("src/crash-5152.rla -o crash6.exr", failureok = True)
27+
command += oiiotool("src/crash-5159.rla -o crash7.exr", failureok = True)
2628

2729
outputs = [ "rlacrop.rla", 'out.txt' ]

testsuite/rla/src/crash-5152.rla

922 Bytes
Binary file not shown.

testsuite/rla/src/crash-5159.rla

1.04 KB
Binary file not shown.

0 commit comments

Comments
 (0)