Move the decompression bomb detection in the TIFF reader into into
ImageInput::check_compression_ratio() so it can be used uniformly for
other input types as well.
Assisted-by: Claude Code / Claude Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
A Softimage PIC file stores a chain of channel packets, each carrying
its own bit depth (8 or 16). The reader assumed one uniform per-channel
size for the whole image, so a file mixing depths (e.g. 16-bit R with
8-bit G) wrote past the end of the scanline buffer.
Fix: always expose one uniform pixel format, the widest depth among the
file's packets, and promote narrower channels to it on read via exact
bit replication (v*257 for 8->16), which preserves the normalized value
exactly. This keeps the pixel layout naturally aligned; representing
channels with their true differing formats instead would pack them at
non-uniform, sometimes misaligned byte offsets.
Assisted-by: Claude Code / claude-sonnet-5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
read_layer_record computed a layer's width/height as `std::abs((int)right
- (int)left)`. The rectangle coordinates are signed 32-bit values (layers
can have negative offsets), but were stored in unsigned (uint32_t)
struct fields, and subtracting two ints near the extremes overflows --
undefined behavior that UBSan flagged on a fuzzed file (1615681375 -
-1605526113 cannot be represented in type 'int').
Retype Layer's and MaskData's top/left/bottom/right fields as int32_t,
matching what they actually hold, instead of reinterpreting bits at the
point of use. Widen to int64_t before subtracting so the computation
can't overflow; the absolute difference of two int32 values always fits
in uint32, and validate_resolution still rejects implausibly large
extents.
The mask data rect (used for layer masks) had the identical
std::abs((int)right - (int)left) overflow hazard in load_layer_channel
and gets the same fix.
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Read10bitFilled() unpacks a scanline of 10-bit "filled" (Method A/B)
samples backwards, and for 1-channel images applies a workaround that
swaps the first and third datum of each group of 3 packed samples (so
column order comes out right). The swap didn't check that a full group
of 3 remained in bounds. Guard the swap so it only fires when a complete
group of 3 remains.
Add a regression test in testsuite/dpx.
Assisted-by: Claude Code / Sonnet 5
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Find OpenCV via its exported CMake config (PREFER_CONFIG) so OpenCV 5 is
located correctly.
Modify our imagebufalgo_opencv.h to work with OpenCV 5. But how to test
whether we did it correctly?
We never had an actual test for the OpenCV interop. So set one up.
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
One small improvement in how decode_exif detects corruption, to ensure
that the initial header examination doesn't read past the array bounds
-- by starting off with a check that the exif blob's size is at least a
big as a TIFFHeader, otherwise parsing it is pointless.
Then the rest of the changes are finding places where readers call
decode_exif but ignore the results, effectively ignoring corrupt Exif
blobs. That is in fact the default intended behavior (if the rest of the
image file can be read properly), but we are supposed to be checking the
global "imageinput:strict" attribute and considering it a hard error
when in strict mode. There were several places where we didn't handle
that case, in the JPEG, PNG, WebP, and Heif readers.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
read_native_tiles() on a tiled EXR whose dimensions aren't a multiple of
the tile size wrote past the caller's buffer when reading an edge tile
range: both readers used a row stride padded up to a whole number of
tiles (nxtiles*tile_width) instead of the caller's actual rectangle
width, overrunning the buffer whenever the last tile column is partial.
Fixed in both exrinput_c.cpp (Core reader) and exrinput.cpp (classic
reader) to use the requested rectangle width as the destination row
stride instead. exrinput.cpp additionally needs the pre-clamp width
specifically, since read_native_tile() forwards a tile-aligned xend past
the image edge for ordinary full-tile reads.
Adds testsuite/openexr-partialtile, a compiled regression test that
calls read_native_tiles() directly for both readers -- the only path
that exercises this code -- and checks pixels and buffer bounds.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
(The same problem and set of fixes were applied to both ImageInput and
ImageOutput. For brevity, we will only discuss ImageInput below.)
ImageInput error messages are stored in a thread_local map keyed by a
unique per-instance id (m_id). The destructor's erase of that entry was
commented out (over caution about the static destruction order fiasco),
so every ImageInput that accumulated an error and was destroyed without
its error being cleared via geterror() leaked one map entry. The id is
monotonically increasing, so entries never get reused and the map grows
without bound.
How did we find this? A fuzzer opening hundreds of thousands of corrupt
files -- each of which errors, and each ImageInput::create() attempt
tries several readers -- grew the map to tens of MB and eventually
OOMed. The leak was traced to this issue.
So we re-enable the erase, guarded against the destruction order fiasco:
the thread_local map is wrapped in a struct whose destructor clears an
'alive' flag, and the ImageInput destructor only touches the map while
it is alive. Each ImageInput removes only its own entry (keyed by its
unique id); other inputs' errors are untouched.
Cross-thread cleanup is still a known limitation -- errors accumulated
in a different thread than the one that descroys the ImageInput are
still leaked, noted in a TODO. However, this should be exceptionally
rare, since not only do you need the two-thread situation, but the one
that made the call that produced an error needs to not check for errors
and return/clear the message. So only badly behaving user code that
fails to check and retrieve errors should have this minor leak. And we
are in the process of adding compile-time warnings for not capturing the
error status values from our APIs.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
A fuzzed JPEG2000 caused a heap-buffer-overflow (caught by ASan) when
copy_scanline read past the end of openjpeg's decoded component buffer.
The file's image canvas was 32x32, but one of its three components had a
horizontal subsampling factor dx=249 (larger than the entire image).
OIIO derives the ImageSpec size from the union of each component's
x0+w*dx / y0+h*dy window, so that bogus component inflated the spec to
249x32. copy_scanline then iterated x over the inflated width while
indexing a component whose data array only held 1 column, walking off
the end. The per-sample guard was also broken: it compared the component
row index against reference-grid offsets (wrong units) and used `>`
instead of `>=` on the column.
Two fixes:
1. Detect the corruption at open(): the JPEG2000 canvas (x1-x0, y1-y0)
is the authoritative image size, and a component's subsampling factor
must be nonzero and no larger than the canvas. Reject files that violate
this instead of decoding a nonsensical, inflated image.
2. Harden copy_scanline by bounds-checking each access against the
actual component array geometry and skip the channel (zero-fill) when
comp.data is null or a subsampling factor is zero, which also closes a
latent divide-by-zero on comp.dx/comp.dy.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Previously, we did not have a PtexInput::valid_file() method, and our
open() method just directly called PtexTexture::open(), which is really
bad with validity checks and can crash or OOM with corrupted files.
But Ptex has a fixed 64 byte header (cheap to read and inspect) that has
lots of fields that let us more accurately check for reasonable file
validity. Therefore, we both implement valid_file, and also have open()
do these checks (rejecting many forms of corruption) before turning the
job over to the Ptex library itself.
Assisted-by: Claude Code / opus-4.8
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Change several build-time options from `option()` to our `set_option()`,
which mostly has the effect of allowing them to be overridden by
environment variables of the same name, which is really handy for CI
variants.
In the process, realized that we had some real problems with handling of
EMBEDPLUGINS=0, it probably hasn't worked properly for years and went
unnoticed because the one CI job variant that was trying to test
EMBEDPLUGINS=0 actually *was not* because it was trying to do it by
setting the environment variable, unaware that it could not be used to
set the option until this patch. So there is a bit of cmake and linkage
fixing that needed to be done to get it squared away.
Assisted-by: Claude Code / opus-4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
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>
Validate channel-row byte availability before interleaving scanlines in
PSD reads, and fail gracefully with an error instead of risking
out-of-bounds access on malformed files.
Assisted-by: GitHub Copilot / GPT-5.3-Codex
Signed-off-by: Larry Gritz <lg@larrygritz.com>
- Add recently published security advisories to SECURITY.md
- Revision of security policy and definitions of vulnerability.
- Also some minor forward-ports and adjustments of CHANGES.
The substantive change is explaining that our policy is to only consider
vulnerabilities if they (a) allow remote code execution, privilege
escalation, and other very severe problems (not crashes, OOM, or most
"DoS" scenaries), and (b) are exploitable in practice by untrusted
actors merely by presenting malicious routine input such as image files
(misuse of APIs, for example, is not a vulnerability).
Everything else should be reported through the usual Issue/PR mechanism,
and not use the security vulnerability reporting regime, which is
massively more overhead for the maintainers.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
PRs #5260 and #5254 interact: #5254 exposed the default type arg on the
module getattribute (`"type"_a = TypeUnknown`), so a one-arg call now
returns None instead of raising TypeError. #5260's test still expected
the old pybind behavior. Update the test and ref to the intended new
behavior.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Run check_open on JXL basic-info dimensions/channels before decode
output allocation to prevent oversized allocation paths on malformed
input.
Also update ref output for libjxl 0.12.0.
Assisted-by: GitHub Copilot / GPT-5.3-Codex
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Find corruptions by validating layer resolutions.
* Find corruptions when eof is hit in the middle of a string.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Truncating a corrupt uint32 length_tab entry into `int` could yield a
huge negative value, causing a runaway allocation and crash in
uncompress_rle_channel. Validate offset/length against the file size
instead.
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
GifSplitPalette() computed `numPixels * (splitElt - firstElt)` as a
32-bit int before dividing. For any image over roughly 16.9M pixels
(e.g. 4117x4117), that intermediate multiplication overflows INT_MAX and
is undefined behavior; in practice it produced a bogus negative/garbage
split count that could drive later indexing into `image` out of its
bounds.
Do the multiply in 64 bits and narrow back afterward -- the quotient is
always <= numPixels, so the narrowing itself is safe.
Assisted-by: Claude Code / Sonnet 5
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
TBHD width/height/channel fields were never cross-checked against how
much tile data the file actually contains, so a corrupt header could
claim an arbitrarily large image (e.g. 16k x 16k) backed by only a few
hundred bytes of real data. readimg() would then resize() a
multi-gigabyte buffer for a tiny malformed file, hanging (or very slowly
succeeding) instead of failing -- a resource-exhaustion DoS on untrusted
input, found via fuzzing.
Bound the claimed image size against how much tile data remains in the
file, generously allowing for RLE's worst-case 128x expansion ratio, and
fail fast if it's implausible.
Assisted-by: Claude Code / claude-sonnet-5
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
CineonInput::open() accepted any per-channel bit depth in [1,32], but
libcineon's ComponentDataSize()/ComponentByteCount() only recognize
{8,10,12,16,32,64} and assert(0) on anything else. A crafted file with
an unsupported bit depth (e.g. 26) passed the OIIO-side check and then
aborted deep inside the vendored library instead of failing cleanly.
Tighten the check to libcineon's actual whitelist so bogus bit depths
are rejected with a proper errorfmt() before ever reaching the vendored
code. Adds a regression fixture (broken_bitdepth2.cin) to the cineon
testsuite.
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
open_raw set imgdata.rawparams.max_raw_memory_mb *after* calling
m_processor->unpack(), so LibRaw's allocation cap never protected the
unpack step -- which is exactly where LibRaw allocates the raw image
buffer. A corrupt raw file claiming huge dimensions could therefore
allocate unbounded and OOM (showing up on the fuzzer as a 0-length crash
artifact from an out-of-memory trip rather than a normal reproducer).
Move the max_raw_memory_mb assignment to immediately after open_file
succeeds, before unpack(), so the cap (default 2048 MB) actually bounds
the allocation. LibRaw then fails the file cleanly instead of OOMing.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
We had missed a case where bit_convert needed outputs to go to the
scratch buffer: PHOTOMETRIC_SEPARATED, but not requesting raw color.
(Because the real number of channels differ from what we are presenting
to the caller.)
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
10MB was conservatively chosen as big enough that anything larger is
definitely bogus for an ICC, Exif, or XMP block in a PSD file.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
read_native_scanlines declared task_set before the locals its queued
decompression lambdas capture (ok, compressed_scratch). On the
leftover-scanline early-return path those locals were destroyed before
~task_set() waited for the queued work, so workers could write through a
dangling ok and hand a freed compressed buffer to zlib.
Declare task_set after the captured locals so its destructor wait runs
while they are still alive, and propagate async decompression failures
via return ok instead of unconditionally returning true.
Assisted-by: Claude Code / opus-4.8
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
validate_header() let the oiio:RawColor/psd:RawData path return true
before the switch that rejects unknown m_header.color_mode values, so an
out-of-range or "hole" (undocumented) color mode reached setup(), which
unconditionally indexes the fixed 10-entry mode_channel_count /
mode_channel_names tables with the attacker-controlled value -- a global
out-of-bounds read (and, per external report, a path to unbounded/bogus
allocation) reachable via the public RawColor config attribute on a
crafted PSD.
Move the "is this a color mode we know about at all" check ahead of the
RawColor early return so it always runs, and keep the existing
Duotone/Lab-unsupported-without-raw check as a second pass after the
early return, preserving current behavior for every valid mode.
Assisted-by: Claude Code / claude-sonnet-5
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
An 18-byte TGA file consisting of just a header (no pixel data) can
claim an arbitrary width/height/bpp -- e.g. a 1472x12000 24bpp image --
and readimg() would allocate the full ~53MB buffer and only discover the
truncation once the first read hits EOF. Worse, the resulting
partially-formed ImageSpec could still propagate to a downstream writer
before the read failure was acted on.
Cross-check the header's width/height/bpp against how many bytes of
pixel data the file could plausibly contain before committing to the
allocation, mirroring the existing pattern in iff.imageio and
jpeg2000.imageio. The check compares against the *on-disk* pixel size
(palette/16-bit storage is smaller per pixel than the decoded, unpacked
ImageSpec) and allows up to 128x expansion for RLE-compressed images (a
1-byte run-length count can encode up to 128 output pixels).
Updated testsuite/targa/ref/out.txt: several existing corrupt-file
regression cases (crash1.tga, crash6.tga, crash3952.tga) are now caught
earlier with clearer messages, and two real oiio-images TGA files
(UCM8.TGA, UTC16.TGA) that use palette/16-bit storage smaller than their
decoded size continue to read correctly.
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
PngReadCallback passed input->geterror(false).c_str() to
png_chunk_error() when a PNG read failed. png_chunk_error() does not
return -- it longjmp's back to libpng's setjmp point, which skips C++
destructors, so the temporary std::string returned by geterror() was
never freed. The fuzzer surfaced this as a small, intermittent leak (a
few KB across hundreds of thousands of runs).
Copy the message into a plain stack buffer (no destructor) before
calling png_chunk_error, so nothing heap-allocated is alive across the
longjmp.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
The mask-based uncompressed decode path copies m_Bpp bytes into a
uint32_t pixel variable. m_Bpp is validated up to 16 (for 128-bit DXGI
formats), but the legacy fmt.masks[] are uint32_t and can only address
32 bits. A corrupt file with m_Bpp > 4 caused a stack-buffer-overflow.
Clamp the copy to sizeof(pixel) = 4 bytes, which is all the masks can
reference anyway.
Assisted-by: Claude Code / claude-sonnet-4-6
Signed-off-by: Larry Gritz <lg@larrygritz.com>
A DDS file with a garbage pixel-format bpp field (e.g. fmt.bpp ~2.5e9)
yields a native bytes-per-pixel (m_Bpp) of hundreds of millions.
internal_readimg() then does `new uint8_t[width * m_Bpp]`, a multi-GB
allocation that OOMs. The core resolution/imagesize limits do not catch
this: they only see the small output ImageSpec, not the native pixel
size, and the existing fmt.bpp check is gated on RGB/LUM/YUV/ALPHA flags
that a corrupt header can simply omit.
Validate m_Bpp for uncompressed formats right after it is computed in
open(): the largest real uncompressed DDS/DXGI pixel is 128-bit (16
bytes), so anything outside [1,16] is a corrupt header and is rejected
before any allocation. The check is confined to uncompressed formats
because m_Bpp is neither set nor used for compressed (block) formats,
which size their data via GetStorageRequirements.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Detect and reject corrupt chunk sizes, flags, and channel configurations
in the header.
Assisted-by: Claude Code / Claude Opus 4.8
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Somehow the check_open validation was only in exrinput.cpp (using the
old C++ AP), but not exrinput_c.cpp (using the C "Core API").
Also adjust the allowable z resolution to reflect that OpenEXR doesn't
support volumetric images. (This was obviously a cut and paste error
long ago.)
Signed-off-by: Larry Gritz <lg@larrygritz.com>
read_offset_tables() sized the RLE start/length tables from the raw
header (`ysize * zsize`), but read_native_scanline() indexes them by the
spec dimensions (`height * nchannels`, derived from the image's
dimension field). A corrupt header (e.g. dimension=1 with zsize=0) makes
`ysize*zsize` = 0 so the tables are empty, while indexing still uses
`height*nchannels` = 1 -- a null-pointer dereference and crash.
Size the tables by `m_spec.height * m_spec.nchannels`, the same quantity
used to index them, so the two can never desync. For well-formed files
this equals ysize*zsize, so behavior is unchanged; a corrupt file now
fails cleanly when the (too-short) table read hits EOF. Also guard
against non-positive table sizes, use vector::data() instead of `&v[0]`
(UB on empty), and fix an off-by-one in the scanline bounds check (y ==
height would index start_tab[-1]).
Add the fuzzer reproducer as testsuite/sgi/src/broken02.sgi.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Upgrade the embedded/vendored pugixml to 1.16, also the default version
that we use for CI with build_pugixml.bash.
Raise PugiXML minimum to from 1.8 to 1.11 (late 2020, over 5 years ago).
Rely completely on pugixml exported cmake config, eliminate our
Findpugixml.cmake. This is possible because we raised our minimum
pugixml to 1.11, which is the point where its exported cmake configs
were reliable.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Add scripts for openjph auto-build when not found (and
OpenImageIO_BUILD_MISSING_DEPS specifies it, or "all"). Build openjph
0.30.1 when missing.
Since we can build it when missing, take away the various missing
openjph allowances from our CI; auto-build when needed for all variants.
More thorough exception catching for openjph in our jpeg2000input.cpp.
When I compile openjph with clang (including icx), I get some runtime
exceptions. Not when I build it with gcc! I don't have the time or
inclination to investigate why, so for our CI, rig it to build openjph
with gcc even when I'm building OIIO with clang/icx.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Circular IFD chain loop detection (identified by for CVE-2022-3970) was
fixed in libtiff by MR 386
(https://gitlab.com/libtiff/libtiff/-/merge_requests/386), released in
libtiff 4.5.0.
Raise the recommended minimum from 4.2 to 4.5 so users on older
installations get a build-time advisory. The hard minimum stays at 4.0
(for now) to preserve compatibility.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
IOMemReader::pread guarded its memcpy with `size + size_t(offset) >
m_buf.size()`. For a large or negative offset, that addition wraps
around size_t and can compare as in-bounds, skipping both the error path
and the size clamp, so the memcpy reads past (or before) the buffer.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When creating an ImageInput, if the initial guess (based on the file
extension) fails to open, we try all the others in turn.
This tries to make that a little cheaper by not doing a full open() but
instead first checking for valid_file, which in theory is a much less
expensive approximate check.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
read_native_scanline computed the scanline's file position as
m_bmp_header.offset + y * m_padded_scanline_size and seeked there
without checking it against the file size. A corrupt header (bogus
pixel-data offset or scanline size) can place that position far outside
the file.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
- Detect corrupted userbuf size.
- Protection against integer multiply overflow in various places in the
DPX reading code.
- Also fix memory load alignment issue. Didn't seem to be a problem in
practice, but the sanitizers weren't happy on some platforms.
Assisted-by: Claude Code / sonnet-4.6
Signed-off-by: Larry Gritz <lg@larrygritz.com>
IBA::make_texture() only passes the full metadata to the first level of
a MIPmap, and that might include output behavior hints, too! Make this
hint "sticky" in the TIFF writer, so if any subimage's first level's
spec enables half output, all the MIP levels of that subimage will allow
it.
Fixes#5239
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Carelessly written hdr/rgbe files, or correctly written ones that are
checked out by git on windows if you're not careful, might use Windows
CRLF line endings. Be silently tolerant of that.
Assisted-by: Claude Code / opus-4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
We were allocating a temporary buffer big enough for the RGBA but
neglecting the z. Only for the 16 bit case.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Guard against corrupted images, which if they said they had more than
the 8 channels allowed by Cineon format, would overrun array bounds
while reading the file. And check for valid bit depths.
Also opportunistically fix a bug I spotted where a red channel was
incrementing the count of greyscale channels rather than the count of
red channels -- which must have been a copy and paste error from
literally 15 years ago. (But those counters are only used for channel
naming, and only if there is more than one greyscale or red channel in
the file. So I doubt anybody has ever hit this bug, or ever would have
in the future.)
Fix possible memory leak by having init() call `m_cin.SetInStream()`
with a null handle to clear out the internals.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When copy-constructing an ImageCache-backed ImageBuf, the "cache-based
or deep" branch set `m_bufspan = {}`, a default-constructed image_span
whose strides are all zero. retile() addresses a pixel within a cache
tile as `offset = pixelindex * m_bufspan.xstride()`, so a zero xstride
made every pixel resolve to tile offset 0 -- the whole copied image read
back as a constant color (the value of pixel 0,0).
Fixes#5243
Added a regression test (test_copy_of_imagecache_backed) that copies a
cache-backed buffer with distinct per-pixel values and verifies the copy
does not read back a constant.
Assisted-by: Claude Code / Claude Opus 4.8
Signed-off-by: Larry Gritz <lg@larrygritz.com>
FITS files are allowed to have multiple "header blocks".
Unfortunately, as originally written, our fits input used *recursion* to
read them. If there is a file with too many header blocks, it can crash
with a stack overflow.
Change to use a loop instead of recursion. And put a maximum iteration
limit of 10000 headers in one file before giving up.
(Note to reviewers: the diff looks bigger than it really is. By
surrounding the contents of read_fits_header with a `for` loop, it
reindented it, and that is the vast majority of the change. I wish
GitHub review diffs more clearly visualized which changed lines are only
whitespace changes.)
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Bump the "bleeding edge" test to C++26 (from 23), and the "latest
releases" Linux test to C++23 (from 20). There are still other variants
that test C++20 including anything based on the VFX2026 platform.
Just one tiny change needed to compile in C++26 mode, related to a
deprecated std type_traits template.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
python_tests was from 2010 and either was checked in inadvertently, or
has long, long been supplanted by our real testsuite feature.
The various TRAVIS_SKIP_LSAN file used to mark certain tests as
skippable when running the leak sanitizer... but we long ago removed the
code in runtest.py that recognized its presence and took action. So it
has served no purpose for quite some time.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Also make our CONTRIBUTING guide link to, and be consistent with,
our SECURITY.md policy file.
Follow OpenEXR's lead by explaining briefly that we only consider
something to be a security issue if it can plausibly be triggered by
ordinary inputs coming from untrusted parties. For example, a
maliciously crafted input image may be a vulnerability, but incorrect
use of an API call is not, because only trusted parties should have
direct access to the APIs.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Some minor fixes and inconsistencies.
- Per the ASWF [repo best practices
guide](https://tac.aswf.io/project_best_practices_guides/repo.html), the
project's technical charter should NOT be redundantly stored in our
repo, but instead we should just link to the canonical copy that the
ASWF maintains.
- Fix incorrect relaive path to CONTRIBUTING.md.
- Fix leftover "interim" designations from TSC members.
- Remove Lewis Wakeland from the TSC roster (he retired some months
back, but somehow we neglected to remove him from the file)
- Add a link to the Google doc with meeting notes.
- Remove stray reference to a 2024 release.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
According to C++26, some newly required std container hardening for
std::span and std::string_view were not reflected in our own analogous
types. In particular, some more precondition checking was required for
span::subspan, first, last, and string_view::remove_prefix and
remove_suffix in order to match the expectations of true parity with
with the std versions.
This is important because some day, we will probably discard our own
string_view and span for the std ones (especially when we are using
compilers in which they are fully hardened).
I found a couple spots where we used remove_prefix and remove_suffix
that assumed the range would be clamped, as it is in our version, rather
than contractually require them to be valid ranges as in std. This would
eventually fail if replaced by std::string_view, so fixing now.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Bump the "bleeding edge" variant to gcc-16, and the "latest releases"
variant to gcc-15.
One minor fix needed to make gcc16 happy -- it is better than earlier
versions at discovering variables whose values are not used.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Mostly for the sake of bots (but also humans) reading plugin.h and
trying to figure out exactly what each function does, just plainly state
which Unix-like dlopen/dlsym/etc function it is the equivalent of.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
It was throwing things off to set CMAKE_CXX_COMPILER in this way,
wrecking the ability to just run as a utility from the command line
without it being set. It was not necessary, as far as I can tell.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Adds agent configuration supporting Claude Code, Cursor, GitHub
Copilot, OpenAI Codex, and Opencode. A single AGENTS.md and contents
of .agents subdirectory serve as the canonical source for
instructions, skills, and other things that can be used across
multiple tools.
A script can be invoked by users to do the local setup of links or
references to allow them to use a particular tool:
`.agents/setup-agent TOOL` Currently supported tools by this script
include Claude Code, Codex, Cursor, GitHub Copilot, and OpenCode.
This initial commit includes two skills that I wrote and have used for
the last couple monthly releases: release-notes-update and
prepare-patch-release. I assume that we will add more over time as we
discover tasks that we want to make standard skill automation for.
Assisted-by: Claude Code / claude-sonnet-4-6
The AGENTS.md, Claude skills, and general scheme are entirely from me.
I used Claude to help me establish the setup script for the other
tools I don't happen to use.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Fixes#5207
Sometimes the `WxH+X+Y` and `+X+Y` notation (borrowed from X11!) can be
a little awkward. This patch extends the accepted syntax to allow commas
to separate the offset components, for example also accepting `WxY,X,Y`
and `X,Y`.
This affects the commands
--create
--crop
--cut
--fit
--fullsize
--origin
--originoffset
--paste
--pattern
--printstats
--resize
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This patch optionally frees the contents of the ustring table as the
process is exiting. It's triggered one of two ways:
- `OIIO::attribute("ustring:cleanup", 1);`
- Environment variable `OIIO_USTRING_CLEANUP=1`
If neither is used, the teardown won't happen, thus avoiding any
pointless work when it should be exiting the process.
The purpose of this is that when using certain debugging tools, the
ustring table (which only grows and never frees) looks like a memory
leak, even though it's by design. Someimes, it's useful to make it not
appear ot leak, and you are happy for a large table to spend time
pointlessly freeing all the data.
Fixes#3913
Signed-off-by: Larry Gritz <lg@larrygritz.com>
In main (working toward the late-2026/2027 releases) these 2022
containers are both outside the promised 3 year support window, and also
are using CentOS 7-based containers that are essentially irrelevant now
that all the studios have moved to RHEL9 (and Rocky/Alma) compatible OS
distros. So there is really no need to continue CI testing on them for
branches aimed at the future.
Note that we still have Ubuntu-based "oldest gcc" and "oldest clang"
jobs, so dropping this does not actually impact our ability to test
against the oldest gcc, C++, and python that we claim to support.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
New release of libheif, which continues to live up to its habit of
changing pixel values ever so slightly with each release.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This message is triggered by the check that the tag of an auto-build
dependency matches the SHA hash we use as a cross-check. This patch is
just amending the error message to reveal the way to override the check,
so people (like me) don't have to go hunting for it if they are
purposely trying to build from a nonstandard commit of a dependency.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
I recently discovered that OCIO's built-in configs' "file rules" that
come into play when using getColorSpaceFromFilepath actually do not
incorporate the search for color space name embedded in the filename
into the logic, and moreover, assume that files ending in ".exr" are
ACES2065-1, of all things. This seems an unfortunate choice because in
the real world, almost no exr files people will encounter actually use
the ap0 primaries.
This PR does the following to shore up this and related edge cases:
* For OCIO built-in configs, fully replace the existing file rules with
a blank slate that just uses the path parsing rule. Everything else is
categorized as "unknown". (Though many of our format readers already do
something better, setting the metadata if they actually know what the
file is.)
* Recognize "unknown" as equivalent to having an empty color space, an
error if you pass such a color space name to IBA color transform
functions.
* For all IBA color conversion, if the input space is not color managed,
be sure NOT TO mark the output color space as the original `to` request!
Input raw means output is also raw.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This is an emergency fix on top of #5201.
PR #5201 passed CI completely. Then I merged it, and now it fails. How
did that happen?
OOOOOOH. The imagespeed_test.cpp is not compiled at all for CI! Only for
local builds. So a build break in it will not be caught in CI. OK,
fixing that, too. It doesn't run in CI, it's not that kind of test, it's
a special benchmarking test meant to run locally and needs to be told a
specific image file to test with. But at least build it for CI so we
can't inadvertently break it again.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Our CI job that builds against VFX Platform 2023 via docker container
aswf/ci-osl:2023-clang15 broke after some dependency changes in a recent
aswf docker container update. It's not worth trying to "fix" an old
year, just lock down to the previous version of the container that is
known to work.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Background: OCIO 1.x configs were LUT based and so tended to have
different color space names for different data types/bit depths. We
capitalized on this to let `oiiotool --autocc` also select type/bits,
but this trick doesn't work so well for OCIO 2.x configs with procedural
(non-LUT) transformations that all look like "float". We don't want to
promote to float just because of this!
Solution: When using color space data type (nearly always float for new
configs and certainly the built-in configs) to infer output, DO NOT use
"float" as a real request. Treat it as not being a specific type, don't
let it override the intended output type for --autocc. (But a color
space still marked as a particular integer bit depth is still an
--autocc inferred request.)
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Add `ColorConfig::isData(name)` method to C++ and Python APIs.
* Fix `ColorConfig.isColorSpaceLinear()` that was missing from the
Python bindings. (Tests added as well.)
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
In PR #5164, we changed to not be quite as aggressive in treating OCIO
exceptions as real errors. All still good with that, but sometimes it is
helpful to know exactly what OCIO wants to tell us, for debugging.
In this PR, we allow our color debug mode (triggered when environment
variable OIIO_DEBUG_COLOR is set to nonzero) to print debugging messages
that reveal the exception messages. It's a debugging aid for developers,
no users will want this. In ordinary operations, we really do want to
just ignore those exceptions. But if something is misbehaving and we
don't know why, it may be helpful to know which of the exceptions is
being caught and what the message was.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
We test on 3 Windows runner configurations currently -- Windows 2022
with VS2022, Windows 2025 with VS2022, and Windows 2025 with VS2026.
GitHub says the middle one will go away imminently (actually, it won't
"break", but it will redirect to Windows 2025 with VS2026, which we
already test). So take it out of our lineup, it's just waste.
The one thing the middle one did differently than the others is test
nanobind, so just move that to the VS2026 one.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
iconvert was the very first command line utility for OIIO, and its
syntax was just `iconvert [options] infile outfile`. YEARS later,
oiiotool came along, superceding iconvert with 100x the functionality,
and also differed by requiring `-o` to specify output file.
iconvert still exists for testing and examples -- iconvert uses just
plain old ImageInput + ImageOutput straightforwardly, whereas oiiotool
is a very complex app that's ImageBuf powered. It's often useful for
debugging to test the ImageInput/ImageOutput layer without any extra
complication, or to directly see how going through an ImageBuf might
differ.
The fact that oiiotool requires `-o` to specify an output file, whereas
iconvert considers that an error, is a minor annoyance when switching
back and forth while debugging, and it's really bugging me.
So here it is, a tiny patch to let iconvert silently accept -o, or not,
and do the obviously right thing. A little quality of life gift to
myself.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
We used new/delete for some small allocations, but there was a possible
leak because early returns would skip the delete. But even when not
leaking, this was wasteful. Replace by a stack allocation using
OIIO_ALLOCA outside the loop -- both problems solved.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This just avoids needing to zero out the allocated buffer, since we know
we are allocating it merely to write into it. Uninitialized contents is
ok and faster.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
These are all items fixed and documented in the latest releases. I had
hoped that the CVE reference numbers would be ready before I pushed
those releases out. But let's just say that GitHub's "3 business days to
assign a CVE" claim is aspirational.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* SWAPRGBABytes: convert to from raw pointers to span-based.
* A variety of overflow safety fixes, replace int (32 bit) with size_t
or int64_t and use safe_mult64.
* Use ImageInput::check_open() for dpx files to check for
reasonable/legal resolutions and channel counts
* Comment out more function declarations not used by OIIO.
Aside:
I should say, this code was originally imported/vendored from another
project by Patrick Palmer: https://github.com/PatrickPalmer/dpx
We imagined it might be an ongoing/improving work, so we made some very
minor changes here and there but endeavoured to make as little change as
possible -- even excluding it from our clang-format rules! -- so that we
could diff against the changing dpx project and pull in any changes or
even use it as an external dependency.
But as you can see if you go there, it hasn't had any modifications for
17 years! So we never needed the "feature" of minimizing divergence from
the original. And now I think with the rate of discovery and reporting
of vulnerabilities and bugs accelerating, the pressure is on to make
this code "safer," for example with these changes in this PR.
I think it's time to give up the pretense entirely and just allow
ourselves to fully absorb this code as our own, be unconcerned about
divergence. So after this PR is merged, I expect follow-ons to:
- Once and for all, fully remove the "dead code" parts that we commented
out because they aren't used in OIIO.
- Allow clang-format to process these files and bring them into
formatting unity with the rest of OIIO.
- Convert all the raw pointer use to spans
- Remove redundant code -- functions in the original dpx project that
were functionality equivalent to things already in OIIO -- where we kept
the originals in place for the sake of minimizing divergence. Let's just
use the OIIO ones we use everywhere else, in cases where they already
exist.
- Root out all remaining overflow and bounds issues, some of the new LLM
based tools are really good at finding those.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This will get us through the great Canonical attack of May 2026, where
it seems that `add-apt-repository ppa:strukturag` is still unreliable
even as we are now multiple days overdue to get our releases out.
We will revert this after everything is fixed. I don't like it silently
passing when we think it's testing heif files but it's really not, any
longer than we have to be in that state.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This is mostly to get it to pass when the GHA runners can't
add-apt-repository ppa:strukturag when it's glitchy down. It can fall
back to an older libheif 1.17 that's already on those runners, but needs
a matching reference output for that version. This is it.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Fix bugs when channel order isn't strictly R,G,B,A by using a
`m_channel_map`.
* Detect malformed file with too many channel packets.
* Gracefully handle duplicate channel bits across packets by not
thinking that there are duplicate channels.
* Guard against channel packets that contain no channels.
Assisted-by: Claude Code / Opus 4.6
(mostly for analysis/identification of the potential problems; touching
the code only for the out-of-order channels fix)
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Fix incorrect return check for fread in read_pixels_pure_run_length.
* Detect and report error for unsupported packet type.
* Fix possible infinite loop from malformed RLE packet with a zero
run-count.
* Check for valid y range in read_native_scanline.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Oh dear, somehow our heif reader never correctly overloaded the
current_subimage method, instead inheriting the one from the base class
that always returns 0. That's very bad for a file type that supports
multiple subimages.
Along the way, simplified how we deal with the fact that the "primary"
subimage doesn't need to be first in the file, but we always want to
present it as if it were first. There was a point where I thought that
was related to the bug so I was rewriting it; it turned out to not be
the problem, but I'd still like to keep the cleanup I had done.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
PR #5154 crossed streams with another that turned on more warnings for
unchecked errors. The check isn't necessary here, though.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Larry Gritz