Skip to content

Commit

Permalink
fix: broken pgm having memory access error
Browse files Browse the repository at this point in the history
Fixes 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 <lg@larrygritz.com>
  • Loading branch information
lgritz committed Dec 5, 2024
1 parent 904df59 commit d2e2baa
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 2 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,3 +69,14 @@ 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 --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 --printstats src/bad-4552.pgm
7 changes: 7 additions & 0 deletions testsuite/pnm/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
# SPDX-License-Identifier: Apache-2.0
# https://github.com/AcademySoftwareFoundation/OpenImageIO

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

imagedir = OIIO_TESTSUITE_IMAGEDIR + "/pnm"

for f in [ "bw-ascii.pbm", "bw-binary.pbm",
Expand All @@ -16,3 +18,8 @@
for f in files:
command += info_command (imagedir + "/" + f,
safematch=True, hash=True)

# Damaged files
files = [ "src/bad-4552.pgm" ]
for f in files:
command += info_command (f, extraargs="--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 d2e2baa

Please # to comment.