Skip to content

Commit b71777c

Browse files
committed
fix(rla): lots of additional validity checking and safety (AcademySoftwareFoundation#5094)
* Validity-check resolution of RLA files with check_open. RLA file headers contain int16_t values for left & right (and top/bottom) window coordinate, leading to a maximum resolution of 2^16-1. * Fix potential bug with sign extension in RLE decoding -- if a signed char is -128, negating it can't make signed char 128 (no such thing), so must widen the var to an int. * Fix potential bug by detecting when the number of matte or auxiliary bits is 0, but the number of matte or aux channels, respectively, is not. * Better bounds checking in decode_channel_group. We did the checks before, but after some accesses that would have been out of bounds! Move the checks earlier than all the accesses. It actually looks like was the result of a cut and paste error long ago. * More care in read_native_scanline for checking valid scanline numbers, offset into m_sot, and check whether ioseek succeeded (i.e. whether the offsets loaded from the file are within the range of the size of the file). Code and fixes all are from my own brain, but some of the analysis of which spots have bounds issues were identified in part by conversation with Claude Code Opus 4.6. --------- Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 5efa454 commit b71777c

2 files changed

Lines changed: 44 additions & 18 deletions

File tree

src/rla.imageio/rlainput.cpp

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ class RLAInput final : public ImageInput {
7878

7979
/// Helper: read the RLA header and scanline offset table.
8080
///
81-
inline bool read_header();
81+
bool read_header();
8282

8383
/// Helper: read and decode a single channel group consisting of
8484
/// channels [first_channel .. first_channel+num_channels-1], which
@@ -175,7 +175,7 @@ RLAInput::open(const std::string& name, ImageSpec& newspec)
175175

176176

177177

178-
inline bool
178+
bool
179179
RLAInput::read_header()
180180
{
181181
// Read the image header, which should have the same exact layout as
@@ -201,6 +201,17 @@ RLAInput::read_header()
201201
if (m_rla.NumOfChannelBits == 0)
202202
m_rla.NumOfChannelBits = 8; // apparently, this can happen
203203

204+
if (m_rla.NumOfMatteBits == 0 && m_rla.NumOfMatteChannels > 0) {
205+
errorfmt("{} matte channels but 0 matte bits. Possible corrupt input?",
206+
m_rla.NumOfMatteChannels);
207+
return false;
208+
}
209+
if (m_rla.NumOfAuxBits == 0 && m_rla.NumOfAuxChannels > 0) {
210+
errorfmt("{} aux channels but 0 aux bits. Possible corrupt input?",
211+
m_rla.NumOfAuxChannels);
212+
return false;
213+
}
214+
204215
// Immediately following the header is the scanline offset table --
205216
// one uint32_t for each scanline, giving absolute offsets (from the
206217
// beginning of the file) where the RLE records start for each
@@ -309,6 +320,15 @@ RLAInput::seek_subimage(int subimage, int miplevel)
309320
m_spec.full_x = m_rla.WindowLeft;
310321
m_spec.full_y = m_spec.full_height - 1 - m_rla.WindowTop;
311322

323+
// Check validity of resolutions. The width and height are the difference
324+
// between left and right (and top/down) coordinates, which are both
325+
// int16_t, giving a maximum of 2^16-1 resolution in each direction. And
326+
// the max number of channels is up to 3 color, up to 3 matte, and up to
327+
// 256 auxiliary channels.
328+
if (!check_open(m_spec, { 0, (1 << 16) - 1, 0, (1 << 16) - 1, 0, 1, 0,
329+
3 + 3 + 256 }))
330+
return false;
331+
312332
// set channel formats and stride
313333
int z_channel = -1;
314334
m_stride = 0;
@@ -480,7 +500,7 @@ RLAInput::decode_rle_span(unsigned char* buf, int n, int stride,
480500
{
481501
size_t e = 0;
482502
while (n > 0 && e < elen) {
483-
signed char count = (signed char)encoded[e++];
503+
int count = (signed char)encoded[e++];
484504
if (count >= 0) {
485505
// run count positive: value repeated count+1 times
486506
for (int i = 0; i <= count && n && e < elen;
@@ -578,6 +598,14 @@ RLAInput::decode_channel_group(int first_channel, short num_channels,
578598
}
579599
}
580600

601+
int bytes_per_chan = ceil2(std::max(int(num_bits), 8)) / 8;
602+
if (size_t(offset + (m_spec.width - 1) * pixelsize
603+
+ num_channels * bytes_per_chan)
604+
> m_buf.size()) {
605+
errorfmt("Probably corrupt file (buffer overrun avoided)");
606+
return false; // Probably corrupt? Would have overrun
607+
}
608+
581609
// If we're little endian, swap endianness in place for 2- and
582610
// 4-byte pixel data.
583611
if (littleendian()) {
@@ -604,15 +632,7 @@ RLAInput::decode_channel_group(int first_channel, short num_channels,
604632
// OIIO conventions.
605633
if (num_bits == 8 || num_bits == 16 || num_bits == 32) {
606634
// ok -- no rescaling needed
607-
}
608-
int bytes_per_chan = ceil2(std::max(int(num_bits), 8)) / 8;
609-
if (size_t(offset + (m_spec.width - 1) * pixelsize
610-
+ num_channels * bytes_per_chan)
611-
> m_buf.size()) {
612-
errorfmt("Probably corrupt file (buffer overrun avoided)");
613-
return false; // Probably corrupt? Would have overrun
614-
}
615-
if (num_bits == 10) {
635+
} else if (num_bits == 10) {
616636
// fast, common case -- use templated hard-code
617637
for (int x = 0; x < m_spec.width; ++x) {
618638
uint16_t* b = (uint16_t*)(&m_buf[offset + x * pixelsize]);
@@ -658,7 +678,13 @@ RLAInput::read_native_scanline(int subimage, int miplevel, int y, int /*z*/,
658678
y = m_spec.height - (y - m_spec.y) - 1;
659679

660680
// Seek to scanline start, based on the scanline offset table
661-
ioseek(m_sot[y]);
681+
if (y < 0 || y >= m_spec.height) {
682+
// Invalid scanline
683+
return false;
684+
}
685+
OIIO_DASSERT(m_sot.size() == size_t(m_spec.height));
686+
if (!ioseek(m_sot[y]))
687+
return false;
662688

663689
// Now decode and interleave the channels.
664690
// The channels are non-interleaved (i.e. rrrrrgggggbbbbb...).

testsuite/rla/ref/out.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,19 +307,19 @@ Reading ../oiio-images/rla/imgmake_rgba_nc8.rla
307307
rla:WhitePoint: 0.31, 0.31, 0.316
308308
Comparing "../oiio-images/rla/imgmake_rgba_nc8.rla" and "imgmake_rgba_nc8.rla"
309309
PASS
310-
oiiotool ERROR: read : "../oiio-images/rla/crash1.rla": Read error: malformed RLE record
310+
oiiotool ERROR: read : "../oiio-images/rla/crash1.rla": 3 aux channels but 0 aux bits. Possible corrupt input?
311311
Full command line was:
312312
> oiiotool ../oiio-images/rla/crash1.rla -o crash1.exr
313-
oiiotool ERROR: read : "../oiio-images/rla/crash2.rla": Read error: not enough data in scanline 73, channel 0
313+
oiiotool ERROR: read : "../oiio-images/rla/crash2.rla": 1 aux channels but 0 aux bits. Possible corrupt input?
314314
Full command line was:
315315
> oiiotool ../oiio-images/rla/crash2.rla -o crash2.exr
316-
oiiotool ERROR: read : "src/crash-1629.rla": Read error: malformed RLE record
316+
oiiotool ERROR: read : "src/crash-1629.rla": rla image full/display resolution must be at least 1x1, but the file specified -16639x256x1. Possible corrupt input?
317317
Full command line was:
318318
> oiiotool src/crash-1629.rla -o crash3.exr
319-
oiiotool ERROR: read : "src/crash-3951.rla": Read error: couldn't read RLE data span
319+
oiiotool ERROR: read : "src/crash-3951.rla": rla image full/display resolution must be at least 1x1, but the file specified 3834x-31743x1. Possible corrupt input?
320320
Full command line was:
321321
> oiiotool src/crash-3951.rla -o crash4.exr
322-
oiiotool ERROR: read : "src/crash-1.rla": Probably corrupt file (buffer overrun avoided)
322+
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
325325
Comparing "rlacrop.rla" and "ref/rlacrop.rla"

0 commit comments

Comments
 (0)