fix: broken pnm files with invalid resolution (#4561)

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]>

src/libOpenImageIO/imagebuf.cpp

@@ -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());
@@ -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();
@@ -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;

src/libutil/strutil.cpp

@@ -1669,8 +1669,9 @@ Strutil::stoi(string_view str, size_t* pos, int base)
         }
         if (c >= base)
             break;
-        acc       = acc * base + c;
         anydigits = true;
+        if (OIIO_LIKELY(!overflow))
+            acc = acc * base + c;
         if (OIIO_UNLIKELY(acc > maxval))
             overflow = true;
     }

src/libutil/strutil_test.cpp

@@ -891,6 +891,7 @@ test_numeric_conversion()
     OIIO_CHECK_EQUAL(Strutil::stoi("-12345678901234567890"),
                      std::numeric_limits<int>::min());
     OIIO_CHECK_EQUAL(Strutil::stoi("0x100", nullptr, 16), 256);  // hex
+    OIIO_CHECK_EQUAL(Strutil::stoi("25555555555555555551"), 2147483647);
 
     OIIO_CHECK_EQUAL(Strutil::stoui("hi"), 0);
     OIIO_CHECK_EQUAL(Strutil::stoui("  "), 0);

src/oiiotool/oiiotool.cpp

@@ -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();
@@ -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;

src/pnm.imageio/pnminput.cpp

@@ -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);
@@ -377,6 +379,9 @@ PNMInput::open(const std::string& name, ImageSpec& newspec)
     if (!read_file_header())
         return false;
 
+    if (!check_open(m_spec))  // check for apparently invalid values
+        return false;
+
     newspec = m_spec;
     return true;
 }

testsuite/pnm/ref/out.txt

@@ -69,3 +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

testsuite/pnm/run.py

@@ -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",
@@ -16,3 +18,8 @@
 for f in files:
     command += info_command (imagedir + "/" + f,
                              safematch=True, hash=True)
+
+# Damaged files
+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)

testsuite/pnm/src/bad-4552.pgm

@@ -0,0 +1,4 @@
+P5
+9 1
+255
+�

testsuite/pnm/src/bad-4553.pgm

@@ -0,0 +1,4 @@
+P1
+25555555555555555551
+255
+�