Skip to content

Commit

Permalink
fix: broken pgm having memory access error (AcademySoftwareFoundation…
Browse files Browse the repository at this point in the history
…#4559)

Fixes AcademySoftwareFoundation#4552
Caught during fuzzing with address sanitizer.

The source of the problem was a corrupted/truncated pgm file. Several
minor modifications in this PR shore up various cascading errors that
followed. Not all were directly causal to the sanitizer trigger, in some
cases I fixed what appeared to be related areas.

* In imagebuf.cpp, any time we free the local pixel memory m_pixels,
also explicitly clear the m_bufspan that has a span representation of
the usable memory and its bounds.
* An extra check related to oiiotool --printstats to make sure that the
image is valid before passing along to stats collection.
* In pnminput.cpp, a better error message when we hit a premature end of
file.

With these fixes in place, we seem to get a graceful error message and
exit when running the POC that was provided with the bug report.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Dec 23, 2024
1 parent e2bbc02 commit 65ef9d2
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 65ef9d2

Please sign in to comment.