Skip to content

fix(softimage): Prevent buffer overruns from corrupt RLE #5142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgritz merged 1 commit into from
Apr 22, 2026
Merged

fix(softimage): Prevent buffer overruns from corrupt RLE #5142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/softimage.imageio/softimageinput.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ SoftimageInput::open(const std::string& name, ImageSpec& spec)
close();
return false;
}
// Some validity checking
if (curPacket.size != 8 && curPacket.size != 16) {
errorfmt("Unsupported bits per channel {}", curPacket.size);
close();
return false;
}
m_channel_packets.push_back(curPacket);

// Add the number of channels in this packet to nchannels
Expand All @@ -135,6 +141,12 @@ SoftimageInput::open(const std::string& name, ImageSpec& spec)
// Set the details in the ImageSpec
m_spec = ImageSpec(m_pic_header.width, m_pic_header.height, nchannels,
chanType);

if (!check_open(m_spec, { 0, 65535, 0, 65535, 0, 1, 0, 4 })) {
close();
return false;
}

m_spec.attribute("BitsPerSample", (int)curPacket.size);

m_spec.attribute("softimage:compression", Strutil::join(encodings, ","));
Expand Down Expand Up @@ -333,6 +345,10 @@ SoftimageInput::read_pixels_pure_run_length(
if (fread(&curCount, 1, 1, m_fd) != 1)
return false;

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In read_pixels_pure_run_length, a corrupt file can encode a run length of 0. In that case linePixelCount never advances and the while (linePixelCount < width) loop can become infinite (DoS/hang). Consider treating a 0 count as corruption and returning false (or otherwise guaranteeing forward progress) before applying the clamp.

Suggested change
// A zero-length run would never advance linePixelCount and could
// cause this loop to spin forever on corrupt input.
if (curCount == 0)
return false;

Copilot uses AI. Check for mistakes.
// Clamp to avoid writing past the end of the scanline buffer
if (linePixelCount + curCount > m_pic_header.width)
curCount = m_pic_header.width - linePixelCount;

if (data) {
// data pointer is set so we're supposed to write data there
size_t pixelSize = pixelChannelSize * channels.size();
Comment on lines +350 to 354

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fread result check in the pure RLE path looks incorrect: fread(pixelData, pixelSize, 1, m_fd) returns the number of items read (expected 1), but the code compares it to pixelSize. This will incorrectly fail for pixelSize != 1 (e.g., 16-bit channels or multi-channel packets). Also, if the read fails, the current early return false leaks pixelData; use RAII (e.g., vector/unique_ptr) or ensure cleanup on all exit paths.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -456,6 +472,10 @@ SoftimageInput::read_pixels_mixed_run_length(
longCount = curCount - 127;
}

// Clamp to avoid writing past the end of the scanline buffer
if (linePixelCount + longCount > m_pic_header.width)
longCount = m_pic_header.width - linePixelCount;

Comment on lines 474 to +478

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In read_pixels_mixed_run_length, if curCount == 128 and the subsequent 16-bit longCount read is 0 (corrupt file), linePixelCount won’t advance and the loop can become infinite. Consider validating longCount > 0 after decoding (and before clamping) and treating 0 as corruption.

Copilot uses AI. Check for mistakes.
if (data) {
// data pointer is set so we're supposed to write data there
size_t pixelSize = pixelChannelSize * channels.size();
Comment on lines +475 to 481

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mixed RLE run-length branch allocates pixelData with new[] and returns false directly on read failure, which leaks the allocation for corrupt/truncated files. Consider using RAII (e.g., std::vector<uint8_t>), or ensure pixelData is freed before any early returns.

Copilot uses AI. Check for mistakes.
Expand Down
14 changes: 14 additions & 0 deletions testsuite/softimage/ref/out.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,17 @@ Reading ../oiio-images/softimage/astone64.pic
Stats FiniteCount: 4096 4096 4096
Constant: No
Monochrome: No
src/broken01.pic : 16 x 4, 1 channel, uint8 softimage
SHA-1: 0526ABB83138E5FD9BC07BB83035C89028527301
channel list: Y
BitsPerSample: 8
softimage:compression: "mixed-rle"
Stats Min: 0 (of 255)
Stats Max: 65 (of 255)
Stats Avg: 16.25 (of 255)
Stats StdDev: 28.15 (of 255)
Stats NanCount: 0
Stats InfCount: 0
Stats FiniteCount: 64
Constant: No
Monochrome: Yes
5 changes: 5 additions & 0 deletions testsuite/softimage/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
# SPDX-License-Identifier: Apache-2.0
# https://github.com/AcademySoftwareFoundation/OpenImageIO

redirect = ' >> out.txt 2>&1 '

files = [ "A4.pic", "astone64.pic" ]
for f in files:
command += info_command (OIIO_TESTSUITE_IMAGEDIR + "/" + f, extraargs="--stats")

# Regression testing of error handling and corrupt files
command += info_command ("--stats src/broken01.pic",
info_program="iinfo", failureok=True)
Binary file added testsuite/softimage/src/broken01.pic
View file
Open in desktop
Binary file not shown.
Loading