From 2c64fb1b49f001403ac73c8338091f7841aa2a57 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 30 Jun 2026 21:47:27 -0400 Subject: [PATCH] fix(exr): use rectangle row stride for partial edge tile reads 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 --- src/cmake/testing.cmake | 3 + src/openexr.imageio/exrinput.cpp | 12 +- src/openexr.imageio/exrinput_c.cpp | 9 +- testsuite/openexr-partialtile/CMakeLists.txt | 26 ++++ testsuite/openexr-partialtile/ref/out.txt | 2 + testsuite/openexr-partialtile/run.py | 27 +++++ .../src/read-partial-tiles.cpp | 112 ++++++++++++++++++ 7 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 testsuite/openexr-partialtile/CMakeLists.txt create mode 100644 testsuite/openexr-partialtile/ref/out.txt create mode 100644 testsuite/openexr-partialtile/run.py create mode 100644 testsuite/openexr-partialtile/src/read-partial-tiles.cpp diff --git a/src/cmake/testing.cmake b/src/cmake/testing.cmake index 93aa7345e6..53673eb497 100644 --- a/src/cmake/testing.cmake +++ b/src/cmake/testing.cmake @@ -407,6 +407,9 @@ macro (oiio_add_all_tests) IMAGEDIR openexr-images URL http://github.com/AcademySoftwareFoundation/openexr-images) endif () + # Regression test (compiles its own helper and generates its own image) + # for a partial edge-tile heap overflow in the OpenEXR readers. + oiio_add_tests (openexr-partialtile) # if (NOT DEFINED ENV{${PROJECT_NAME}_CI}) # oiio_add_tests (openexr-damaged # IMAGEDIR openexr-images diff --git a/src/openexr.imageio/exrinput.cpp b/src/openexr.imageio/exrinput.cpp index cc0062af21..ff1b5d439a 100644 --- a/src/openexr.imageio/exrinput.cpp +++ b/src/openexr.imageio/exrinput.cpp @@ -1404,6 +1404,9 @@ OpenEXRInput::read_native_tiles(int subimage, int miplevel, int xbegin, size_t pixelbytes = m_spec.pixel_bytes(chbegin, chend, true); int firstxtile = (xbegin - m_spec.x) / m_spec.tile_width; int firstytile = (ybegin - m_spec.y) / m_spec.tile_height; + // Caller's buffer is sized for [xbegin, xend) before clamping below; that + // width, not the clamped one, is the destination row stride. + int requested_xend = xend; // clamp to the image edge xend = std::min(xend, m_spec.x + m_spec.width); yend = std::min(yend, m_spec.y + m_spec.height); @@ -1447,10 +1450,15 @@ OpenEXRInput::read_native_tiles(int subimage, int miplevel, int xbegin, return false; } if (data != origdata) { - stride_t user_scanline_bytes = (xend - xbegin) * pixelbytes; + // Source rows (temp buffer) are spaced by the padded whole-tile + // stride; destination rows (caller's buffer) are spaced by the + // requested width. Only the valid, clamped columns are copied. + stride_t user_scanline_bytes = (xend - xbegin) * pixelbytes; + stride_t dest_scanline_stride = (requested_xend - xbegin) + * pixelbytes; stride_t scanline_stride = nxtiles * m_spec.tile_width * pixelbytes; for (int y = ybegin; y < yend; ++y) - memcpy((char*)origdata + (y - ybegin) * scanline_stride, + memcpy((char*)origdata + (y - ybegin) * dest_scanline_stride, (char*)data + (y - ybegin) * scanline_stride, user_scanline_bytes); } diff --git a/src/openexr.imageio/exrinput_c.cpp b/src/openexr.imageio/exrinput_c.cpp index 590d7d7077..284e4f6d68 100644 --- a/src/openexr.imageio/exrinput_c.cpp +++ b/src/openexr.imageio/exrinput_c.cpp @@ -1535,14 +1535,19 @@ OpenEXRCoreInput::read_native_tiles(int subimage, int miplevel, int xbegin, * size_t((xend - xbegin + tilew - 1) / tilew)); + // Destination row stride is the caller's requested rectangle width, not + // the padded whole-tile width (would overrun the buffer on a partial edge + // tile) and not the level-clamped width (would pack rows too tightly for + // a valid tile-aligned xend past the edge, per ImageSpec::valid_tile_range). + int requested_xend = xend; + size_t scanlinebytes = size_t(requested_xend - xbegin) * pixelbytes; + xend = std::min(xend, spec.x + levw); yend = std::min(yend, spec.y + levh); zend = std::min(zend, spec.z + spec.depth); int nxtiles = (xend - xbegin + tilew - 1) / tilew; int nytiles = (yend - ybegin + tileh - 1) / tileh; - size_t scanlinebytes = size_t(nxtiles) * size_t(tilew) * pixelbytes; - DBGEXR( "exr rnt {}:{}:{} ({}-{}|{}x{})[{}-{}] -> t {}, {} n {}, {} pb {} sb {} tsz {}x{}\n", m_userdata.m_io->filename(), subimage, miplevel, xbegin, xend, diff --git a/testsuite/openexr-partialtile/CMakeLists.txt b/testsuite/openexr-partialtile/CMakeLists.txt new file mode 100644 index 0000000000..28ebcb9e76 --- /dev/null +++ b/testsuite/openexr-partialtile/CMakeLists.txt @@ -0,0 +1,26 @@ +# Copyright Contributors to the OpenImageIO project. +# SPDX-License-Identifier: Apache-2.0 +# https://github.com/AcademySoftwareFoundation/OpenImageIO + +cmake_minimum_required (VERSION 3.18) +project (oiio-openexr-partialtile + LANGUAGES CXX) + +if (NOT CMAKE_BUILD_TYPE) + set (CMAKE_BUILD_TYPE "Release") +endif () + +set (CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to prefer (17, 20, etc.)") +set (CMAKE_CXX_STANDARD_REQUIRED ON) +set (CMAKE_CXX_EXTENSIONS OFF) + +find_package (OpenImageIO CONFIG REQUIRED) + +# Special for OIIO testsuite when running in sanitize mode +if (DEFINED ENV{SANITIZE}) + add_compile_options (-fsanitize=$ENV{SANITIZE}) + add_link_options (-fsanitize=$ENV{SANITIZE}) +endif () + +add_executable (read-partial-tiles src/read-partial-tiles.cpp) +target_link_libraries (read-partial-tiles PRIVATE OpenImageIO::OpenImageIO) diff --git a/testsuite/openexr-partialtile/ref/out.txt b/testsuite/openexr-partialtile/ref/out.txt new file mode 100644 index 0000000000..6f7abad1a0 --- /dev/null +++ b/testsuite/openexr-partialtile/ref/out.txt @@ -0,0 +1,2 @@ +classic: rect 63x64 smashed=0 baddiffs=0 -> PASS +core: rect 63x64 smashed=0 baddiffs=0 -> PASS diff --git a/testsuite/openexr-partialtile/run.py b/testsuite/openexr-partialtile/run.py new file mode 100644 index 0000000000..dd730bdf61 --- /dev/null +++ b/testsuite/openexr-partialtile/run.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python + +# Copyright Contributors to the OpenImageIO project. +# SPDX-License-Identifier: Apache-2.0 +# https://github.com/AcademySoftwareFoundation/OpenImageIO + +# Regression test for a heap out-of-bounds write when reading a partial edge +# tile of a tiled OpenEXR whose dimensions are not a multiple of the tile size. +# We build a small C++ program (which uses the low-level read_native_tiles() +# API for both the classic and OpenEXR-Core readers) and run it against a +# tiled EXR whose 127x129 size is deliberately not a multiple of the 64x64 +# tile size. + +if platform.system() == 'Windows' : + prefix = ".\\build\\Release\\" +else : + prefix = "./build/" + +# Make a tiled EXR whose dimensions are not a multiple of the tile size. +command += oiio_app("oiiotool") + \ + "--pattern noise:type=uniform:min=0:max=1 127x129 3 -d half " + \ + "--tile 64 64 -o partial.exr > out.txt ;" + +# Build and run the C++ regression test. +command += run_app("cmake -S " + test_source_dir + " -B build -DCMAKE_BUILD_TYPE=Release >> build.txt 2>&1", silent=True) +command += run_app("cmake --build build --config Release >> build.txt 2>&1", silent=True) +command += run_app(prefix + "read-partial-tiles partial.exr") diff --git a/testsuite/openexr-partialtile/src/read-partial-tiles.cpp b/testsuite/openexr-partialtile/src/read-partial-tiles.cpp new file mode 100644 index 0000000000..8934f4325b --- /dev/null +++ b/testsuite/openexr-partialtile/src/read-partial-tiles.cpp @@ -0,0 +1,112 @@ +// Copyright Contributors to the OpenImageIO project. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/AcademySoftwareFoundation/OpenImageIO + +// Regression test for a heap out-of-bounds write in the OpenEXR readers. +// +// A tiled OpenEXR whose dimensions are not an exact multiple of the tile size +// exposes an edge-tile range whose width is less than a whole number of tiles. +// ImageInput::read_native_tiles() must place the decoded rows into the +// caller's buffer using the requested-rectangle row stride, not a stride +// padded up to a whole number of tiles. Getting this wrong overruns the +// caller buffer (and/or scrambles the pixels) for the partial edge tile. +// +// This test reads such an edge rectangle through the low-level +// read_native_tiles() API into a buffer sized exactly to the rectangle and +// checks both that the pixels match a full-image read and that trailing guard +// bytes are untouched. It exercises both the classic and the OpenEXR-Core +// based readers. + +#include + +#include +#include + +using namespace OIIO; + + +static int +check_reader(const char* filename, int use_core) +{ + OIIO::attribute("openexr:core", use_core); + const char* label = use_core ? "core" : "classic"; + + auto in = ImageInput::open(filename); + if (!in) { + OIIO::print("{}: open failed\n", label); + return 1; + } + const ImageSpec& spec = in->spec(); + int W = spec.width, H = spec.height, C = spec.nchannels; + + // Work in the file's native pixel format, comparing raw bytes so the test + // is agnostic to the channel type. Full image via read_image, as ground + // truth. + size_t pixelbytes = spec.pixel_bytes(true); + std::vector full(size_t(W) * H * pixelbytes); + if (!in->read_image(0, 0, 0, C, TypeDesc::UNKNOWN, full.data())) { + OIIO::print("{}: read_image failed\n", label); + return 1; + } + + // The right-edge column of tiles: [lastxtile*tw, W) x [0, tile_height). + int tw = spec.tile_width, th = spec.tile_height; + int xbegin = spec.x + ((W - 1) / tw) * tw; + int xend = spec.x + W; + int ybegin = spec.y; + int yend = spec.y + std::min(th, H); + int rw = xend - xbegin, rh = yend - ybegin; + + size_t rowbytes = size_t(rw) * pixelbytes; + size_t expected = rowbytes * size_t(rh); + + // Exact rectangle buffer plus trailing guard bytes. + const size_t guard = 256; + std::vector buf(expected + guard, 0xCC); + bool ok = in->read_native_tiles(0, 0, xbegin, xend, ybegin, yend, spec.z, + spec.z + 1, 0, C, buf.data()); + if (!ok) { + OIIO::print("{}: read_native_tiles failed: {}\n", label, + in->geterror()); + return 1; + } + + // Guard bytes must be pristine. + size_t smashed = 0; + for (size_t i = expected; i < buf.size(); ++i) + if (buf[i] != 0xCC) + ++smashed; + + // Each rectangle row must match the corresponding slice of the full image. + int baddiffs = 0; + for (int y = 0; y < rh; ++y) { + const unsigned char* trow = buf.data() + size_t(y) * rowbytes; + const unsigned char* frow = full.data() + + (size_t(ybegin - spec.y + y) * W + + (xbegin - spec.x)) + * pixelbytes; + for (size_t i = 0; i < rowbytes; ++i) + if (trow[i] != frow[i]) + ++baddiffs; + } + + OIIO::print("{}: rect {}x{} smashed={} baddiffs={} -> {}\n", label, rw, rh, + smashed, baddiffs, + (smashed == 0 && baddiffs == 0) ? "PASS" : "FAIL"); + in->close(); + return (smashed == 0 && baddiffs == 0) ? 0 : 1; +} + + +int +main(int argc, char** argv) +{ + if (argc < 2) { + OIIO::print("usage: read-partial-tiles file.exr\n"); + return 2; + } + int err = 0; + err += check_reader(argv[1], 0); // classic reader + err += check_reader(argv[1], 1); // OpenEXR-Core reader + return err ? 1 : 0; +}