From 8f423c9975206c2dfcba85f8407fcc125883d5cc Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Wed, 23 Aug 2023 13:32:09 -0700 Subject: [PATCH] fix: various protections against corrupted files (#3954) Fixes #3952 Several interrelated fixes to guard against corrupted files: * Extend the `limits:imagesize_MB` sanity check to Targa files to address #3952 specifically, and check in that failing example file into the testsuite. (N.B.: Only a couple readers use this, TIFF and now Targa. it should be extended to the others. Mackerel!) * Improve the error message used for the above, both for Targa and TIFF. * For low-memory situations, perhaps the default of 32GB is too large. Make the default be the minimum of 32GB and 1/2 the size of physical memory on the machine. (An app that legit expects humongous files is expected to raise the limit appropriately.) * The generic "Read error: hit end of file" we sometimes issued from `ImageInput::ioread()` was hard to discern which reader it came from, in cases where the file extension was absent or misleading and a succession of files were tried. By changing this error message to say which format reader triggered the error, it might make these errors easier to track down in the future. This also changed the reference output of a few test involving truncated or corrupted files. --------- Signed-off-by: Larry Gritz --- src/include/OpenImageIO/imageio.h | 15 +++++++++------ src/libOpenImageIO/imageinput.cpp | 3 ++- src/libOpenImageIO/imageio.cpp | 3 ++- src/targa.imageio/targainput.cpp | 14 ++++++++++++++ src/tiff.imageio/tiffinput.cpp | 7 +++++-- testsuite/dds/ref/out.txt | 2 +- testsuite/png-damaged/ref/out.txt | 14 +++++++------- testsuite/targa/ref/out.txt | 11 ++++++++--- testsuite/targa/run.py | 5 ++++- testsuite/targa/src/crash3952.tga | Bin 0 -> 21 bytes 10 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 testsuite/targa/src/crash3952.tga diff --git a/src/include/OpenImageIO/imageio.h b/src/include/OpenImageIO/imageio.h index cca4b2b976..117c5ac88c 100644 --- a/src/include/OpenImageIO/imageio.h +++ b/src/include/OpenImageIO/imageio.h @@ -2895,12 +2895,15 @@ OIIO_API std::string geterror(bool clear = true); /// /// - `int limits:imagesize_MB` (32768) /// -/// When nonzero, the maximum size in MB of the uncompressed pixel data of -/// a single 2D image. Images whose headers indicate that they are larger -/// than this might be assumed to be corrupted or malicious files. The -/// default is 32768 (32 GB of uncompressed pixel data -- equivalent to 64k -/// x 64k x 4 channel x half). In situations when images larger than this -/// are expected to be encountered, you should raise this limit. +/// When nonzero, the maximum expected size in MB of the uncompressed pixel +/// data of a single 2D image. Images whose headers indicate that they are +/// larger than this might be assumed to be corrupted or malicious files. +/// The default is 32768 (32 GB of uncompressed pixel data -- equivalent to +/// 64k x 64k x 4 channel x half), or the total amount of total physical +/// memory available to the running process, whichever is smaller. In +/// situations when images larger than this are expected to be encountered, +/// you should raise this limit. Setting the limit to 0 means having no +/// limit. /// /// - `int log_times` /// diff --git a/src/libOpenImageIO/imageinput.cpp b/src/libOpenImageIO/imageinput.cpp index 2e32d24010..003ca4f57d 100644 --- a/src/libOpenImageIO/imageinput.cpp +++ b/src/libOpenImageIO/imageinput.cpp @@ -1249,7 +1249,8 @@ ImageInput::ioread(void* buf, size_t itemsize, size_t nitems) size_t n = m_io->read(buf, size); if (n != size) { if (size_t(m_io->tell()) >= m_io->size()) - ImageInput::errorfmt("Read error: hit end of file"); + ImageInput::errorfmt("Read error: hit end of file in {} reader", + format_name()); else ImageInput::errorfmt( "Read error at position {}, could only read {}/{} bytes {}", diff --git a/src/libOpenImageIO/imageio.cpp b/src/libOpenImageIO/imageio.cpp index cc1cd357d6..6455b50b1a 100644 --- a/src/libOpenImageIO/imageio.cpp +++ b/src/libOpenImageIO/imageio.cpp @@ -46,7 +46,8 @@ int tiff_half(0); int tiff_multithread(1); int dds_bc5normal(0); int limit_channels(1024); -int limit_imagesize_MB(32 * 1024); +int limit_imagesize_MB(std::min(32 * 1024, + int(Sysutil::physical_memory() >> 20))); ustring font_searchpath(Sysutil::getenv("OPENIMAGEIO_FONTS")); ustring plugin_searchpath(OIIO_DEFAULT_PLUGIN_SEARCHPATH); std::string format_list; // comma-separated list of all formats diff --git a/src/targa.imageio/targainput.cpp b/src/targa.imageio/targainput.cpp index 0a4e781166..4a46d5ea0c 100644 --- a/src/targa.imageio/targainput.cpp +++ b/src/targa.imageio/targainput.cpp @@ -15,6 +15,7 @@ #include #include +#include "imageio_pvt.h" #include "targa_pvt.h" OIIO_PLUGIN_NAMESPACE_BEGIN @@ -285,6 +286,19 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) } m_spec.attribute("targa:version", int(m_tga_version)); + if (pvt::limit_imagesize_MB + && m_spec.image_bytes(true) + > pvt::limit_imagesize_MB * imagesize_t(1024 * 1024)) { + errorfmt( + "Uncompressed image size {:.1f} MB exceeds the {} MB limit.\n" + "Image claimed to be {}x{}, {}-channel {}. Possible corrupt input?\n" + "If this is a valid file, raise the OIIO attribute \"limits:imagesize_MB\".", + float(m_spec.image_bytes(true)) / float(1024 * 1024), + pvt::limit_imagesize_MB, m_spec.width, m_spec.height, + m_spec.nchannels, m_spec.format); + return false; + } + if (m_spec.alpha_channel != -1 && m_alpha_type == TGA_ALPHA_USEFUL && m_keep_unassociated_alpha) m_spec.attribute("oiio:UnassociatedAlpha", 1); diff --git a/src/tiff.imageio/tiffinput.cpp b/src/tiff.imageio/tiffinput.cpp index 21a119d523..4e9ad50f22 100644 --- a/src/tiff.imageio/tiffinput.cpp +++ b/src/tiff.imageio/tiffinput.cpp @@ -849,9 +849,12 @@ TIFFInput::seek_subimage(int subimage, int miplevel) && m_spec.image_bytes(true) > pvt::limit_imagesize_MB * imagesize_t(1024 * 1024)) { errorfmt( - "Uncompressed image size {:.1f} MB exceeds \"limits:imagesize_MB\" = {}. Possible corrupt input?\nIf you're sure this is a valid file, raise the OIIO global attribute \"limits:imagesize_MB\".", + "Uncompressed image size {:.1f} MB exceeds the {} MB limit.\n" + "Image claimed to be {}x{}, {}-channel {}. Possible corrupt input?\n" + "If this is a valid file, raise the OIIO attribute \"limits:imagesize_MB\".", float(m_spec.image_bytes(true)) / float(1024 * 1024), - pvt::limit_imagesize_MB); + pvt::limit_imagesize_MB, m_spec.width, m_spec.height, + m_spec.nchannels, m_spec.format); return false; } return true; diff --git a/testsuite/dds/ref/out.txt b/testsuite/dds/ref/out.txt index da6bc5de3e..15e9cfeca4 100644 --- a/testsuite/dds/ref/out.txt +++ b/testsuite/dds/ref/out.txt @@ -184,7 +184,7 @@ Reading ../oiio-images/dds/broken/dds_bc3_just_header.dds channel list: R, G, B, A compression: "DXT5" textureformat: "Plain Texture" -oiiotool ERROR: read : "../oiio-images/dds/broken/dds_bc3_no_full_header.dds": Read error: hit end of file +oiiotool ERROR: read : "../oiio-images/dds/broken/dds_bc3_no_full_header.dds": Read error: hit end of file in dds reader Full command line was: > oiiotool --info -v -a --hash ../oiio-images/dds/broken/dds_bc3_no_full_header.dds Reading ../oiio-images/dds/broken/dds_bc7_just_header.dds diff --git a/testsuite/png-damaged/ref/out.txt b/testsuite/png-damaged/ref/out.txt index c7755df5e0..7f276f6bc9 100644 --- a/testsuite/png-damaged/ref/out.txt +++ b/testsuite/png-damaged/ref/out.txt @@ -1,11 +1,11 @@ -libpng error: IDAT: Read error: hit end of file +libpng error: IDAT: Read error: hit end of file in png reader iconvert ERROR copying "../oiio-images/png/broken/invalid_gray_alpha_sbit.png" to "invalid_gray_alpha_sbit.png" : - Read error: hit end of file -PNG read error: IDAT: Read error: hit end of file + Read error: hit end of file in png reader +PNG read error: IDAT: Read error: hit end of file in png reader libpng error: No IDATs written into file Comparing "../oiio-images/png/broken/invalid_gray_alpha_sbit.png" and "invalid_gray_alpha_sbit.png" -libpng error: tEXt: Read error: hit end of file -libpng error: tEXt: Read error: hit end of file +libpng error: tEXt: Read error: hit end of file in png reader +libpng error: tEXt: Read error: hit end of file in png reader idiff ERROR: Could not read invalid_gray_alpha_sbit.png: - Invalid image file "invalid_gray_alpha_sbit.png": Read error: hit end of file -PNG read error: tEXt: Read error: hit end of file + Invalid image file "invalid_gray_alpha_sbit.png": Read error: hit end of file in png reader +PNG read error: tEXt: Read error: hit end of file in png reader diff --git a/testsuite/targa/ref/out.txt b/testsuite/targa/ref/out.txt index 6fae97d737..cc6eb69cba 100644 --- a/testsuite/targa/ref/out.txt +++ b/testsuite/targa/ref/out.txt @@ -188,14 +188,14 @@ Comparing "../oiio-images/targa/round_grill.tga" and "round_grill.tga" PASS Converting src/crash1.tga to crash1.exr iconvert ERROR copying "src/crash1.tga" to "crash1.exr" : - Read error: hit end of file + Read error: hit end of file in targa reader oiiotool ERROR: read : "src/crash2.tga": Palette image with no palette Full command line was: > oiiotool --oiioattrib try_all_readers 0 src/crash2.tga -o crash2.exr oiiotool ERROR: read : "src/crash3.tga": Palette image with no palette Full command line was: > oiiotool --oiioattrib try_all_readers 0 src/crash3.tga -o crash3.exr -oiiotool ERROR: read : "src/crash6.tga": Read error: hit end of file +oiiotool ERROR: read : "src/crash6.tga": Read error: hit end of file in targa reader Full command line was: > oiiotool --oiioattrib try_all_readers 0 src/crash6.tga -o crash6.exr oiiotool ERROR: read : "src/crash1707.tga": Invalid alpha type 138. Corrupted header? @@ -203,7 +203,12 @@ Full command line was: > oiiotool --oiioattrib try_all_readers 0 src/crash1707.tga -o crash1707.exr oiiotool ERROR: read : "src/crash1708.tga": Corrupt palette index Full command line was: -> oiiotool --oiioattrib try_all_readers 0 src/crash1708.tga -o crash1707.exr +> oiiotool --oiioattrib try_all_readers 0 src/crash1708.tga -o crash1708.exr +oiiotool ERROR: read : "src/crash3952.tga": Uncompressed image size 15231.5 MB exceeds the 1024 MB limit. +Image claimed to be 60927x65535, 4-channel uint8. Possible corrupt input? +If this is a valid file, raise the OIIO attribute "limits:imagesize_MB". +Full command line was: +> oiiotool --oiioattrib limits:imagesize_MB 1024 --oiioattrib try_all_readers 0 src/crash3952.tga -o crash3952.exr Reading src/1x1.tga src/1x1.tga : 1 x 1, 3 channel, uint2 targa SHA-1: DB9237F28F9622F7B7E73B783A8822B63BDB3708 diff --git a/testsuite/targa/run.py b/testsuite/targa/run.py index 2f14b4622c..1baba221b9 100755 --- a/testsuite/targa/run.py +++ b/testsuite/targa/run.py @@ -23,7 +23,10 @@ command += oiiotool("--oiioattrib try_all_readers 0 src/crash5.tga -o crash5.exr", failureok = True) command += oiiotool("--oiioattrib try_all_readers 0 src/crash6.tga -o crash6.exr", failureok = True) command += oiiotool("--oiioattrib try_all_readers 0 src/crash1707.tga -o crash1707.exr", failureok = True) -command += oiiotool("--oiioattrib try_all_readers 0 src/crash1708.tga -o crash1707.exr", failureok = True) +command += oiiotool("--oiioattrib try_all_readers 0 src/crash1708.tga -o crash1708.exr", failureok = True) +command += oiiotool("--oiioattrib limits:imagesize_MB 1024 " + "--oiioattrib try_all_readers 0 " + "src/crash3952.tga -o crash3952.exr", failureok = True) # Test odds and ends, unusual files command += rw_command("src", "1x1.tga") diff --git a/testsuite/targa/src/crash3952.tga b/testsuite/targa/src/crash3952.tga new file mode 100644 index 0000000000000000000000000000000000000000..a5ad0103a92277180f89d2824bdbaea62edefdb2 GIT binary patch literal 21 ccmZQzU}E46Ffz1I;4=CD_Wyqc0|f;I05S9h^8f$< literal 0 HcmV?d00001