Skip to content

Commit

Permalink
fix: broken pnm files with invalid resolution (#4561)
Browse files Browse the repository at this point in the history
Fixes #4553

Caught during fuzzing with address sanitizer. The file appeared to have
a resolution so big it would not be able to satisfy the memory
allocation.

Solution: add the check_open to take an early abort if resolutions are
bigger than could possibly be valid.

Also have Strutil::stoi hande 32 bit overflow without UB overflow that
the sanitizer complains about (that was the other cascading error that
this same test case encountered in the sanitizer after the bad
allocation).

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Dec 6, 2024
1 parent 78927ac commit 6396f7c
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/libOpenImageIO/imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ ImageBufImpl::new_pixels(size_t size, const void* data)
// consider this an uninitialized ImageBuf, issue an error, and hope
// it's handled well downstream.
m_pixels.reset();
m_bufspan = make_span<std::byte>(nullptr, 0);
OIIO::debugfmt("ImageBuf unable to allocate {} bytes ({})\n", size,
e.what());
error("ImageBuf unable to allocate {} bytes ({})\n", size, e.what());
Expand Down Expand Up @@ -720,6 +721,8 @@ ImageBufImpl::free_pixels()
m_allocated_size = 0;
}
m_pixels.reset();
// print("IB Freed pixels of length {}\n", m_bufspan.size());
m_bufspan = make_span<std::byte>(nullptr, 0);
m_deepdata.free();
m_storage = ImageBuf::UNINITIALIZED;
m_blackpixel.clear();
Expand Down Expand Up @@ -806,6 +809,7 @@ ImageBufImpl::clear()
m_spec = ImageSpec();
m_nativespec = ImageSpec();
m_pixels.reset();
m_bufspan = make_span<std::byte>(nullptr, 0);
m_localpixels = nullptr;
m_spec_valid = false;
m_pixels_valid = false;
Expand Down
4 changes: 3 additions & 1 deletion src/oiiotool/oiiotool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5801,7 +5801,8 @@ action_printstats(Oiiotool& ot, cspan<const char*> argv)
auto options = ot.extract_options(command);
bool allsubimages = options.get_int("allsubimages", ot.allsubimages);

ot.read();
if (!ot.read())
return;
ImageRecRef top = ot.top();

print_info_options opt = ot.info_opts();
Expand All @@ -5818,6 +5819,7 @@ action_printstats(Oiiotool& ot, cspan<const char*> argv)
opt.roi.chend);
}
std::string errstring;
OIIO_ASSERT(top.get());
print_info(std::cout, ot, top.get(), opt, errstring);

ot.printed_info = true;
Expand Down
4 changes: 3 additions & 1 deletion src/pnm.imageio/pnminput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,10 @@ PNMInput::read_file_scanline(void* data, int y)
numbytes = m_spec.nchannels * 4 * m_spec.width;
else
numbytes = m_spec.scanline_bytes();
if (size_t(numbytes) > m_remaining.size())
if (size_t(numbytes) > m_remaining.size()) {
errorfmt("Premature end of file");
return false;
}
buf.assign(m_remaining.begin(), m_remaining.begin() + numbytes);

m_remaining.remove_prefix(numbytes);
Expand Down
11 changes: 11 additions & 0 deletions testsuite/pnm/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ Reading ../oiio-images/pnm/test-3.pfm
oiio:ColorSpace: "Rec709"
pnm:bigendian: 1
pnm:binary: 1
Reading src/bad-4552.pgm
oiiotool ERROR: -info : SHA-1: Premature end of file
Full command line was:
> oiiotool --info -v -a --hash --oiioattrib try_all_readers 0 --printstats src/bad-4552.pgm
src/bad-4552.pgm : 9 x 1, 1 channel, uint8 pnm
channel list: Y
oiio:ColorSpace: "Rec709"
pnm:binary: 1
oiiotool ERROR: read : "src/bad-4552.pgm": Premature end of file
Full command line was:
> oiiotool --info -v -a --hash --oiioattrib try_all_readers 0 --printstats src/bad-4552.pgm
oiiotool ERROR: read : "src/bad-4553.pgm": pnm image resolution may not exceed 65535x65535, but the file appears to be 2147483647x255. Possible corrupt input?
Full command line was:
> oiiotool --info -v -a --hash --oiioattrib try_all_readers 0 --printstats src/bad-4553.pgm
2 changes: 1 addition & 1 deletion testsuite/pnm/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@
safematch=True, hash=True)

# Damaged files
files = [ "src/bad-4553.pgm" ]
files = [ "src/bad-4552.pgm", "src/bad-4553.pgm" ]
for f in files:
command += info_command (f, extraargs="--oiioattrib try_all_readers 0 --printstats", failureok=True)
4 changes: 4 additions & 0 deletions testsuite/pnm/src/bad-4552.pgm
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
P5
9 1
255

0 comments on commit 6396f7c

Please sign in to comment.