Skip to content

Commit

Permalink
feat(api): ImageInput::open_check (#3967)
Browse files Browse the repository at this point in the history
Not unlike the ImageOutput::open_check we added a few months back, this
creates an ImageInput utility function that centralizes a number of
validation checks. Currently, this is primarily a check on allowed
resolution and total size, aimed at making a guess at files that are
corrupted or malicious.

Also adds a new `supports("noimage")` query.

---------

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Aug 30, 2023
1 parent b008158 commit fd8e64a
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 31 deletions.
6 changes: 4 additions & 2 deletions src/fits.imageio/fits_pvt.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ class FitsInput final : public ImageInput {
int supports(string_view feature) const override
{
return (feature == "arbitrary_metadata"
|| feature == "exif" // Because of arbitrary_metadata
|| feature == "iptc"); // Because of arbitrary_metadata
|| feature == "exif" // Because of arbitrary_metadata
|| feature == "iptc" // Because of arbitrary_metadata
|| feature == "noimage" // allow metadata only, no pixels
);
}
bool valid_file(const std::string& filename) const override;
bool open(const std::string& name, ImageSpec& spec) override;
Expand Down
59 changes: 58 additions & 1 deletion src/include/OpenImageIO/imageio.h
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,10 @@ class OIIO_API ImageInput {
/// Does this format reader support retrieving a reduced
/// resolution copy of the image via the `thumbnail()` method?
///
/// - `"noimage"` :
/// Does this format allow 0x0 sized images, i.e. an image file
/// with metadata only and no pixels?
///
/// This list of queries may be extended in future releases. Since this
/// can be done simply by recognizing new query strings, and does not
/// require any new API entry points, addition of support for new
Expand Down Expand Up @@ -1818,6 +1822,53 @@ class OIIO_API ImageInput {
/// Helper: retrieve the current position of the proxy, akin to ftell.
int64_t iotell() const;

/// Helper: convenience boilerplate for several checks and operations that
/// every implementation of ImageInput::open() will need to do. Failure is
/// presumed to indicate a file that is corrupt (or perhaps maliciously
/// crafted) and no further reading should be attempted.
///
/// @param spec
/// The ImageSpec that we are validating.
///
/// @param range
/// An ROI that describes the allowable pixel coordinates and channel
/// indices as half-open intervals. For example, the default value
/// `{0, 65535, 0, 65535, 0, 1, 0, 4}` means that pixel coordinates
/// must be non-negative and the width and height be representable by
/// a uint16 value, up to 4 channels are allowed, and volumes are not
/// permitted (z coordinate may only be 0). File formats that can
/// handle larger resolutions, or volumes, or >4 channels must
/// override these limits!
///
/// @param flags
/// A bitfield flag (bits defined by `enum OpenChecks`) that can
/// indicate additional checks to perform, or checks that should be
/// skipped.
///
/// @returns
/// Return `true` if the spec is valid and passes all checks,
/// otherwise return `false` and make appropriate calls to
/// this->errorfmt() to record the errors.
///
/// Checks performed include:
///
/// * Whether the resolution and channel count are within the range
/// implied by `range`.
/// * Whether the channel count is within the `"limit:channels"` OIIO
/// attribute.
/// * The total uncompressed pixel data size is expected to be within the
/// `"limit:imagesize_MB"` OIIO attribute.
///
bool check_open (const ImageSpec &spec,
ROI range = {0, 65535, 0, 65535, 0, 1, 0, 4},
uint64_t flags = 0);

/// Bit field definitions for the `flags` argument to `check_open()`.
enum class OpenChecks : uint64_t {
Defaults = 0,
// Reserved for future use
};

/// @}

private:
Expand Down Expand Up @@ -2526,7 +2577,8 @@ class OIIO_API ImageOutput {
/// is the request to write volumetric data but the format writer
/// doesn't support it).
///
/// Returns true if ok, false if the open request can't be satisfied.
/// Returns true if ok, false if the open request can't be satisfied (and
/// makes appropriate calls to this->errorfmt() to record the errors).
///
/// Having a central helper method for this is beneficial:
///
Expand Down Expand Up @@ -2562,6 +2614,11 @@ class OIIO_API ImageOutput {
/// indicate additional checks to perform, or checks that should be
/// skipped.
///
/// @returns
/// Return `true` if the spec is valid and passes all checks,
/// otherwise return `false` and make appropriate calls to
/// this->errorfmt() to record the errors.
///
/// Checks performed include:
///
/// * Whether the open `mode` is one allowed by `supports("multiimage")`
Expand Down
59 changes: 59 additions & 0 deletions src/libOpenImageIO/imageinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1286,4 +1286,63 @@ ImageInput::iotell() const
}



bool
ImageInput::check_open(const ImageSpec& spec, ROI range, uint64_t /*flags*/)
{
// Check for sensible resolutions, etc.
if ((m_spec.width <= 0 || m_spec.height <= 0 || m_spec.depth <= 0
|| m_spec.nchannels <= 0)
&& !supports("noimage")) {
errorfmt(
"{} image resolution must be at least 1x1, but the file specified {}x{}. Possible corrupt input?",
format_name(), m_spec.width, m_spec.height);
return false;
}
if (m_spec.depth > 1) {
if (m_spec.width > range.width() || m_spec.height > range.height()
|| m_spec.depth > range.depth()) {
errorfmt(
"{} image resolution may not exceed {}x{}x{}, but the file appears to be {}x{}x{}. Possible corrupt input?",
format_name(), range.width(), range.height(), range.depth(),
m_spec.width, m_spec.height, m_spec.depth);
return false;
}
} else {
if (m_spec.width > range.width() || m_spec.height > range.height()) {
errorfmt(
"{} image resolution may not exceed {}x{}, but the file appears to be {}x{}. Possible corrupt input?",
format_name(), range.width(), range.height(), m_spec.width,
m_spec.height);
return false;
}
}
if (spec.nchannels > range.nchannels()) {
errorfmt(
"{} does not support {}-channel images. Possible corrupt input?",
format_name(), spec.nchannels);
return false;
}
if (pvt::limit_channels && spec.nchannels > pvt::limit_channels) {
errorfmt(
"{} channels exceeds \"limits:channels\" = {}. Possible corrupt input?\nIf you're sure this is a valid file, raise the OIIO global attribute \"limits:channels\".",
spec.nchannels, pvt::limit_channels);
return false;
}
if (pvt::limit_imagesize_MB
&& 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;
}

return true; // all is ok
}

OIIO_NAMESPACE_END
12 changes: 1 addition & 11 deletions src/targa.imageio/targainput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,18 +286,8 @@ 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);
if (!check_open(m_spec))
return false;
}

if (m_spec.alpha_channel != -1 && m_alpha_type == TGA_ALPHA_USEFUL
&& m_keep_unassociated_alpha)
Expand Down
19 changes: 2 additions & 17 deletions src/tiff.imageio/tiffinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -839,24 +839,9 @@ TIFFInput::seek_subimage(int subimage, int miplevel)
errorfmt("No support for data format of \"{}\"", m_filename);
return false;
}
if (pvt::limit_channels && m_spec.nchannels > pvt::limit_channels) {
errorfmt(
"{} channels exceeds \"limits:channels\" = {}. Possible corrupt input?\nIf you're sure this is a valid file, raise the OIIO global attribute \"limits:channels\".",
m_spec.nchannels, pvt::limit_channels);
if (!check_open(m_spec,
{ 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 1 << 16 }))
return false;
}
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;
}
return true;
} else {
std::string e = oiio_tiff_last_error();
Expand Down

0 comments on commit fd8e64a

Please sign in to comment.