From 78927acce09bd4fd7d4d7dccf7b7d4eff4295fcf Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 6 Dec 2024 11:48:26 -0800 Subject: [PATCH] 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 --- 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