From d0aa9b41feec0be0683ff9cc532cba4f854fa8bb Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Fri, 26 Jun 2026 22:06:00 -0400 Subject: [PATCH] fix(jpeg2000): reject corrupt component geometry; bounds-check copy_scanline A fuzzed JPEG2000 caused a heap-buffer-overflow (caught by ASan) under `iinfo -stats`: copy_scanline read past the end of openjpeg's decoded component buffer. Root cause: the file's image canvas is 32x32, but one of its three components reports a horizontal subsampling factor dx=249 (a step 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 as defense in depth: bounds-check each access against the actual component array geometry -- row in [0, comp.h) and column in [0, comp.w) -- 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. Verified that the openjpeg conformance suite (jpeg2000-j2kp4files), which includes chroma-subsampled images, still decodes correctly. Assisted-by: Claude Code / Claude Opus 4.8 Signed-off-by: Larry Gritz --- src/jpeg2000.imageio/jpeg2000input.cpp | 42 +++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/jpeg2000.imageio/jpeg2000input.cpp b/src/jpeg2000.imageio/jpeg2000input.cpp index 3217fa631e..e3f0daa097 100644 --- a/src/jpeg2000.imageio/jpeg2000input.cpp +++ b/src/jpeg2000.imageio/jpeg2000input.cpp @@ -581,6 +581,14 @@ Jpeg2000Input::open(const std::string& name, ImageSpec& p_spec) return false; } + // The JPEG2000 canvas (x0,y0)-(x1,y1) is the authoritative image size. + // Each component samples that canvas at its own subsampling factor. + const unsigned int canvas_w = (m_image->x1 > m_image->x0) + ? m_image->x1 - m_image->x0 + : 0; + const unsigned int canvas_h = (m_image->y1 > m_image->y0) + ? m_image->y1 - m_image->y0 + : 0; for (int c = 0; c < channelCount; ++c) { const opj_image_comp_t& comp(m_image->comps[c]); if (!comp.data) { @@ -589,6 +597,20 @@ Jpeg2000Input::open(const std::string& name, ImageSpec& p_spec) close(); return false; } + // Reject corrupt files whose component geometry is inconsistent with + // the image canvas. A subsampling factor must be nonzero and cannot + // exceed the canvas (a step larger than the whole image is not a real + // image); such values otherwise inflate the derived ImageSpec far + // beyond the actual canvas and lead to out-of-bounds reads of the + // decoded component data. + if (comp.dx == 0 || comp.dy == 0 || comp.dx > canvas_w + || comp.dy > canvas_h) { + errorfmt( + "Invalid Jpeg2000 component {} subsampling {}x{} for {}x{} image", + c, comp.dx, comp.dy, canvas_w, canvas_h); + close(); + return false; + } } unsigned int maxPrecision = 0; @@ -784,15 +806,25 @@ Jpeg2000Input::copy_scanline(int y, int /*z*/, void* data) int bits = sizeof(T) * 8; for (int c = 0; c < nc; ++c) { const opj_image_comp_t& comp(m_image->comps[c]); - int chan_ybegin = comp.y0, chan_yend = comp.y0 + comp.h * comp.dy; - int chan_xend = comp.w * comp.dx; - int yoff = (y - comp.y0) / comp.dy; + // The decoded component data is a comp.w x comp.h array of samples, + // possibly subsampled by comp.dx/comp.dy relative to the full image. + // A malformed file can have component geometry that doesn't match the + // image spec, so bounds-check every access against the actual array + // rather than trusting the dimensions to line up. + if (!comp.data || comp.dx == 0 || comp.dy == 0) { + for (int x = 0; x < m_spec.width; ++x) + scanline[x * nc + c] = T(0); + continue; + } + int comp_row = (y - int(comp.y0)) / int(comp.dy); for (int x = 0; x < m_spec.width; ++x) { - if (yoff < chan_ybegin || yoff >= chan_yend || x > chan_xend) { + int comp_col = x / int(comp.dx); + if (comp_row < 0 || comp_row >= int(comp.h) || comp_col < 0 + || comp_col >= int(comp.w)) { // Outside the window of this channel scanline[x * nc + c] = T(0); } else { - unsigned int val = comp.data[yoff * comp.w + x / comp.dx]; + unsigned int val = comp.data[comp_row * comp.w + comp_col]; if (comp.sgnd) val += (1 << (bits / 2 - 1)); scanline[x * nc + c] = (T)bit_range_convert(val, comp.prec,