Skip to content

Commit

Permalink
fix: various protections against corrupted files (#3954)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
lgritz committed Aug 25, 2023
1 parent cd1ffe5 commit 88249cf
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 22 deletions.
15 changes: 9 additions & 6 deletions src/include/OpenImageIO/imageio.h
Original file line number Diff line number Diff line change
Expand Up @@ -2779,12 +2779,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`
///
Expand Down
3 changes: 2 additions & 1 deletion src/libOpenImageIO/imageinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,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 {}",
Expand Down
3 changes: 2 additions & 1 deletion src/libOpenImageIO/imageio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
ustring plugin_searchpath(OIIO_DEFAULT_PLUGIN_SEARCHPATH);
std::string format_list; // comma-separated list of all formats
Expand Down
14 changes: 14 additions & 0 deletions src/targa.imageio/targainput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <OpenImageIO/sysutil.h>
#include <OpenImageIO/typedesc.h>

#include "imageio_pvt.h"
#include "targa_pvt.h"

OIIO_PLUGIN_NAMESPACE_BEGIN
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions src/tiff.imageio/tiffinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -804,9 +804,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;
Expand Down
2 changes: 1 addition & 1 deletion testsuite/dds/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions testsuite/png-damaged/ref/out.txt
Original file line number Diff line number Diff line change
@@ -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
11 changes: 8 additions & 3 deletions testsuite/targa/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -188,22 +188,27 @@ 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?
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
Expand Down
5 changes: 4 additions & 1 deletion testsuite/targa/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Binary file added testsuite/targa/src/crash3952.tga
Binary file not shown.

0 comments on commit 88249cf

Please sign in to comment.