Skip to content

feat: add "limits:resolution" global attribute to cap per-dimension size#5297

Merged
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-limitsres
Jul 11, 2026
Merged

feat: add "limits:resolution" global attribute to cap per-dimension size#5297
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-limitsres

Conversation

@lgritz

@lgritz lgritz commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

The existing "limits:imagesize_MB" rejects files whose total uncompressed size exceeds a cap, but it cannot catch an image that is tiny in one dimension yet absurdly large in another (e.g. an image claiming 32 x 50331680, which slips under the 32 GB default). Add a companion global attribute, "limits:resolution", that caps the maximum number of pixels along any single dimension.

Like imagesize_MB, this is enforced only in ImageInput::check_open(), intended for validity checks when reading untrusted files.

Applications may set the attribute larger or smaller than this default, depending on their need to deal with images larger than these limits, and/or willingness to risk mistaking a corrupted or maliciously crafted image for a legitimate one on this basis.

Now then, with this in place, we can clean up some of the values we used in passing valid resolution ranges to check_open(). That was originally meant to express the true limits of what was valid in the file formats, but had in many cases been conflated with common sense "what value is so big that it's probably bogus". Now we can disentangle these ideas -- the value passed to check_open is the definitional limit of the file format itself, and the "limits:resolution" is the overrideable global policy for what we consider more likely to be malicious/corrupted than merely legit big.

Assisted-by: Claude Code / Claude Opus 4.8

@jessey-git

Copy link
Copy Markdown
Contributor

This new setting is confusing because the existing check_open call already does a rather strict validation of both width and height against the allowed range. This new setting will do nothing when set higher to that specified range - which in some cases are already pretty low.

FWIW I've been finding that the existing check_open calls are conflating both the file format's "valid range" and our hopeful wish at detecting "invalid ranges" E.g. we got tripped up after release at one point due to an internal baking process wanting to produce a 1x4'194'304 EXR (it was a lookup data file really). But that's not permitted because of EXR's 1 << 20 specified check_open limit and it wasn't detected during tests because our test suite typically uses sizes much smaller than that.

@lgritz

lgritz commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Yeah, I hear what you're saying. It is admittedly getting a little confusing.

The checks are all done centrally by check_open(), but the values for the checks come from two places:

  • the format-reading plugin explicitly passes the format-spec-imposed limits, which are supposed to be related to what the file format can legally represent (or that seems sensible for that kind of file)
  • the new global "limits" attributes are a policy decision (which can be set by OIIO::attribute()) about the threshold for which images of any file type are presumed to be fishy -- possibly a legal file, but maybe constructed to try to cause OOM or overflows.

Essentially, the more stringent of those constraints is the operative one.

I'm 100% open to hearing that I've chosen any of these values wrong. Often they were best guesses about "how big are we confident we will never see in real life for a legit file?"

Do you think we shouldn't add the new linear resolution limit at all, and ONLY check the "legal" limits of each file format and the single overall "total memory" limit? Maybe as we finally plug the last of the holes related to int overflow, the linear limits will be less important? I was just trying to find another form of low-hanging fruit heuristics for what looks like an obviously bogus file, but an exr that is a 1x4M lookup table definitely is a counter-point to what I was trying to do.

@jessey-git

Copy link
Copy Markdown
Contributor

Having something like limits:resolution is valuable I think (and we can keep its default at 1M). This would have made the fix we had to do a little less stressful because it's configurable, so if our original fix didn't work out for some reason we'd use it as an escape hatch.

My feedback was just alluding to that if we get this new policy value, it means we should also adjust the check_open calls to be strictly about what the file format supports. So the below calls should all change to use their respective format limits directly:

cineon.imageio\cineoninput.cpp
121:    if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 8 })) {
dicom.imageio\dicominput.cpp
234:    if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 1 << 16 }))
dpx.imageio\dpxinput.cpp
213:    if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 8 }))
dpx.imageio\dpxoutput.cpp
179:    if (!check_open(mode, userspec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 256 }))
hdr.imageio\hdroutput.cpp
203:    if (!check_open(mode, newspec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 3 },
heif.imageio\heifoutput.cpp
106:    if (!check_open(mode, newspec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 4 },
ico.imageio\icoinput.cpp
246:            || !check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 4 })) {
jpeg2000.imageio\jpeg2000output.cpp
188:    if (!check_open(mode, spec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 4 },
openexr.imageio\exrinput.cpp
1128:    if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 1 << 12 }))
openexr.imageio\exrinput_c.cpp
1121:    if (!check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 1 << 12 }))
openexr.imageio\exroutput.cpp
214:                        { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 1 << 12 }))
png.imageio\pnginput.cpp
181:        || !check_open(m_spec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 4 })) {
png.imageio\pngoutput.cpp
137:    if (!check_open(mode, userspec, { 0, 1 << 20, 0, 1 << 20, 0, 1, 0, 4 }))
raw.imageio\rawinput.cpp
315:    if (type.size() >= (1 << 20))
tiff.imageio\tiffinput.cpp
1024:                        { 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 1 << 16 }))
tiff.imageio\tiffoutput.cpp
478:                    { 0, 1 << 20, 0, 1 << 20, 0, 1 << 16, 0, 1 << 16 }))

@lgritz

lgritz commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Yes, I agree! Because we didn't have a configurable "limits:resolution" when we first set up check_open, there are some places where file formats have very permissive actual limits, and we kind of co-mingled common sense limits. Now we have a clean way to separate the two concerns.

OpenEXR, in particular, can go back to a full uint32 range of spec-permitted resolution.

(In your listing above, are you just showing the lines to remind me of their locations, or have you already adjusted each one to be what you think the format supports?)

@jessey-git

Copy link
Copy Markdown
Contributor

Just there as a reminder that those locations probably all need to change. They are all currently set to use 1<<20 at the moment.

@lgritz

lgritz commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

So updated. I added the following explanation to the commit description:

Now then, with this in place, we can clean up some of the values we used in passing valid resolution ranges to check_open(). That was originally meant to express the true limits of what was valid in the file formats, but had in many cases been conflated with common sense "what value is so big that it's probably bogus". Now we can disentangle these ideas -- the value passed to check_open is the definitional limit of the file format itself, and the "limits:resolution" is the overrideable global policy for what we consider more likely to be malicious/corrupted than merely legit big.

The existing "limits:imagesize_MB" rejects files whose total
uncompressed size exceeds a cap, but it cannot catch an image that is
tiny in one dimension yet absurdly large in another (e.g. an image
claiming 32 x 50331680, which slips under the 32 GB default). Add a
companion global attribute, "limits:resolution", that caps the maximum
number of pixels along any single dimension.

Like imagesize_MB, this is enforced only in ImageInput::check_open(),
intended for validity checks when reading untrusted files.

Applications may set the attribute larger or smaller than this
default, depending on their need to deal with images larger than these
limits, and/or willingness to risk mistaking a corrupted or
maliciously crafted image for a legitimate one on this basis.

Now then, with this in place, we can clean up some of the values we
used in passing valid resolution ranges to check_open(). That was
originally meant to express the true limits of what was valid in the
file formats, but had in many cases been conflated with common sense
"what value is so big that it's probably bogus". Now we can
disentangle these ideas -- the value passed to check_open is the
definitional limit of the file format itself, and the
"limits:resolution" is the overrideable global policy for what we
consider more likely to be malicious/corrupted than merely legit big.

Assisted-by: Claude Code / Claude Opus 4.8

Signed-off-by: Larry Gritz <lg@larrygritz.com>

@jessey-git jessey-git left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good now!

@lgritz lgritz merged commit b2d3328 into AcademySoftwareFoundation:main Jul 11, 2026
30 checks passed
@lgritz lgritz deleted the lg-limitsres branch July 11, 2026 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants