From c2c1b28b5d21d901365ab40f153b17ffb8a1cc66 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 3 Dec 2024 14:29:30 -0800 Subject: [PATCH] fix: broken pnm files with invalid resolution 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 --- src/libutil/strutil.cpp | 3 ++- src/libutil/strutil_test.cpp | 1 + src/pnm.imageio/pnminput.cpp | 3 +++ testsuite/pnm/ref/out.txt | 3 +++ testsuite/pnm/run.py | 7 +++++++ testsuite/pnm/src/bad-4553.pgm | 4 ++++ 6 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 testsuite/pnm/src/bad-4553.pgm diff --git a/src/libutil/strutil.cpp b/src/libutil/strutil.cpp index 2b68ed8c1f..531285e4cb 100644 --- a/src/libutil/strutil.cpp +++ b/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; } diff --git a/src/libutil/strutil_test.cpp b/src/libutil/strutil_test.cpp index d2ca5e55cc..fba21ee41f 100644 --- a/src/libutil/strutil_test.cpp +++ b/src/libutil/strutil_test.cpp @@ -891,6 +891,7 @@ test_numeric_conversion() OIIO_CHECK_EQUAL(Strutil::stoi("-12345678901234567890"), std::numeric_limits::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); diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp index 2642a47083..3c9e7d3a4a 100644 --- a/src/pnm.imageio/pnminput.cpp +++ b/src/pnm.imageio/pnminput.cpp @@ -377,6 +377,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; } diff --git a/testsuite/pnm/ref/out.txt b/testsuite/pnm/ref/out.txt index b20de733d5..8c1a957016 100644 --- a/testsuite/pnm/ref/out.txt +++ b/testsuite/pnm/ref/out.txt @@ -69,3 +69,6 @@ Reading ../oiio-images/pnm/test-3.pfm oiio:ColorSpace: "Rec709" pnm:bigendian: 1 pnm:binary: 1 +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 diff --git a/testsuite/pnm/run.py b/testsuite/pnm/run.py index 4fd5a6ed67..76f7297a88 100755 --- a/testsuite/pnm/run.py +++ b/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-4553.pgm" ] +for f in files: + command += info_command (f, extraargs="--oiioattrib try_all_readers 0 --printstats", failureok=True) diff --git a/testsuite/pnm/src/bad-4553.pgm b/testsuite/pnm/src/bad-4553.pgm new file mode 100644 index 0000000000..608017ca8d --- /dev/null +++ b/testsuite/pnm/src/bad-4553.pgm @@ -0,0 +1,4 @@ +P1 +25555555555555555551 +255 +� \ No newline at end of file