From 3e34c79ac2bc129142810a3d41b1a7e7fbc19933 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sun, 26 Jul 2026 15:12:44 -0700 Subject: [PATCH] fix(jpeg2000): guard the HTJ2K reader, not just the OpenJPEG one This plugin has two reader codepaths, one using OpenJPEG and the other using HTJ2K. Earlier bomb-header guards only covered the OpenJPEG path. The HTJ2K's open() function read width and height straight from the header and then allocated the full image before any check ran. The HTJ2K path now runs the same size checks as the OpenJPEG path before it sizes anything from the header. Error handling on this path was also broken. A failed read logged an error but did not stop; the code kept going and used image data that was never filled in. This happened in two decode loops, and also when the codestream failed to open. One buffer resize sat outside its own error check. The read function could then return success after logging failure, so callers copied scanlines out of an empty buffer. Each of these paths now stops on error, and the scanline copy is bounds-checked. Two more small fixes: a file with zero color components used to read past the end of an internal array, so that case is now rejected. And one error path in ICC strict mode used to skip closing the file; it now closes it like every other failure path. Assisted-by: Claude Code / claude-opus-5 Signed-off-by: Larry Gritz --- src/jpeg2000.imageio/jpeg2000input.cpp | 67 +++++++++++++++----- testsuite/htj2k/ref/out.txt | 13 ++++ testsuite/htj2k/run.py | 11 ++++ testsuite/htj2k/src/bomb-ratio.j2c | Bin 0 -> 310 bytes testsuite/htj2k/src/bomb-resolution.j2c | Bin 0 -> 310 bytes testsuite/htj2k/src/make_malformed_htj2k.py | 63 ++++++++++++++++++ testsuite/htj2k/src/valid-16x16.j2c | Bin 0 -> 310 bytes 7 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 testsuite/htj2k/src/bomb-ratio.j2c create mode 100644 testsuite/htj2k/src/bomb-resolution.j2c create mode 100644 testsuite/htj2k/src/make_malformed_htj2k.py create mode 100644 testsuite/htj2k/src/valid-16x16.j2c diff --git a/src/jpeg2000.imageio/jpeg2000input.cpp b/src/jpeg2000.imageio/jpeg2000input.cpp index 1f7f9b8406..6ea0347853 100644 --- a/src/jpeg2000.imageio/jpeg2000input.cpp +++ b/src/jpeg2000.imageio/jpeg2000input.cpp @@ -338,8 +338,13 @@ Jpeg2000Input::ojph_read_header() int ch = siz.get_num_components(); const int w = siz.get_recon_width(0); const int h = siz.get_recon_height(0); - TypeDesc dtype; + TypeDesc dtype = TypeDesc::UINT8; + if (ch < 1) { + errorfmt("No components in HTJ2K codestream"); + close(); + return false; + } if (ch > 4) ch = 4; // Only do the first 4 channels. m_bpp.resize(ch); @@ -347,14 +352,14 @@ Jpeg2000Input::ojph_read_header() for (int c = 0; c < ch; c++) { switch (siz.get_bit_depth(c)) { case 8: - dtype = TypeDesc::UCHAR; + dtype = TypeDesc::UINT8; m_bpp[c] = 1; break; case 10: case 12: case 16: m_bpp[c] = 2; - dtype = TypeDesc::USHORT; + dtype = TypeDesc::UINT16; break; case 32: m_bpp[c] = 4; @@ -374,6 +379,17 @@ Jpeg2000Input::ojph_read_header() } m_spec = ImageSpec(w, h, ch, dtype); + + // The dimensions come straight from the SIZ marker, and ojph_read_image() + // allocates the whole image up front, so the size has to be vetted here -- + // the guards on the OpenJPEG path below do not run for HTJ2K files. + if (!check_open(m_spec, { 0, std::numeric_limits::max(), 0, + std::numeric_limits::max(), 0, 1, 0, 4 }) + || !check_compression_ratio(m_spec, ioproxy() ? ioproxy()->size() : 0)) { + close(); + return false; + } + m_spec.default_channel_names(); m_spec.attribute("oiio:BitsPerSample", siz.get_bit_depth(0)); m_spec.set_colorspace("srgb_rec709_scene"); @@ -395,11 +411,12 @@ Jpeg2000Input::ojph_read_image() const size_t bufsize = clamped_mult64(clamped_mult64(uint64_t(w), uint64_t(h)), clamped_mult64(uint64_t(ch), uint64_t(buffer_bpp))); - m_buf.resize(bufsize); try { + m_buf.resize(bufsize); codestream.create(); - } catch (const std::runtime_error& e) { + } catch (const std::exception& e) { errorfmt("openjph exception {}", e.what()); + return false; } int file_bit_depth = siz.get_bit_depth(0); // Assuming RGBA are the same. @@ -413,20 +430,25 @@ Jpeg2000Input::ojph_read_image() ojph::line_buf* line = nullptr; try { line = codestream.pull(comp_num); - } catch (const std::runtime_error& e) { + } catch (const std::exception& e) { errorfmt("openjph exception {}", e.what()); } + if (!line) { + if (!has_error()) + errorfmt("Could not pull HTJ2K scanline"); + return false; + } const ojph::si32* sp = line->i32; OIIO_DASSERT(int(comp_num) == c); - if (m_spec.format == TypeDesc::UCHAR) { + if (m_spec.format == TypeDesc::UINT8) { unsigned char* dout = &m_buf[i * w * ch]; dout += c; for (int j = w; j > 0; j--, dout += ch) { *dout = *sp++; } } - if (m_spec.format == TypeDesc::USHORT) { + if (m_spec.format == TypeDesc::UINT16) { unsigned short* dout = (unsigned short*)&m_buf[buffer_bpp * (i * w * ch)]; dout += c; @@ -443,20 +465,25 @@ Jpeg2000Input::ojph_read_image() ojph::line_buf* line = nullptr; try { line = codestream.pull(comp_num); - } catch (const std::runtime_error& e) { + } catch (const std::exception& e) { errorfmt("openjph exception {}", e.what()); } + if (!line) { + if (!has_error()) + errorfmt("Could not pull HTJ2K scanline"); + return false; + } const ojph::si32* sp = line->i32; OIIO_DASSERT(int(comp_num) == c); - if (m_spec.format == TypeDesc::UCHAR) { + if (m_spec.format == TypeDesc::UINT8) { unsigned char* dout = &m_buf[i * w * ch]; dout += c; for (int j = w; j > 0; j--, dout += ch) { *dout = *sp++; } } - if (m_spec.format == TypeDesc::USHORT) { + if (m_spec.format == TypeDesc::UINT16) { unsigned short* dout = (unsigned short*)&m_buf[buffer_bpp * (i * w * ch)]; dout += c; @@ -737,11 +764,19 @@ Jpeg2000Input::read_native_scanline(int subimage, int miplevel, int y, int z, #ifdef USE_OPENJPH if (ojph_reader) { - if (!ojph_image_read) - ojph_read_image(); - unsigned char* start - = &m_buf[buffer_bpp * (y * m_spec.width * m_spec.nchannels)]; - memcpy(data, start, buffer_bpp * m_spec.width * m_spec.nchannels); + if (!ojph_image_read && !ojph_read_image()) + return false; + // The whole image is buffered up front, so a failed or short decode + // must not be copied out of regardless. + const int64_t scanline_size = int64_t(buffer_bpp) * m_spec.width + * m_spec.nchannels; + const int64_t offset = scanline_size * y; + if (y < 0 || offset < 0 + || offset + scanline_size > int64_t(m_buf.size())) { + errorfmt("Scanline {} is outside the decoded HTJ2K image", y); + return false; + } + memcpy(data, &m_buf[offset], scanline_size); } else { #endif // USE_OPENJPH diff --git a/testsuite/htj2k/ref/out.txt b/testsuite/htj2k/ref/out.txt index ef3176dd3a..c6d4748900 100644 --- a/testsuite/htj2k/ref/out.txt +++ b/testsuite/htj2k/ref/out.txt @@ -4,3 +4,16 @@ Comparing "../oiio-images/dpx/dpx_nuke_10bits_rgb.dpx" and "testdpx.j2c" PASS Comparing "../oiio-images/tahoe-gps.jpg" and "testcompress.j2c" PASS +oiiotool ERROR: read : "src/bomb-resolution.j2c": jpeg2000 image dimension 2000000000x2000000000 exceeds "limits:resolution" = 1048576 for a single dimension. Possible corrupt input? +If you're sure this is a valid file, raise the OIIO global attribute "limits:resolution". +Full command line was: +> oiiotool --info -v src/bomb-resolution.j2c +oiiotool ERROR: read : "src/bomb-ratio.j2c": jpeg2000 header claims a 28610 MB image from a 310 byte file; probably a corrupt or malicious header +Full command line was: +> oiiotool --info -v src/bomb-ratio.j2c +Reading src/valid-16x16.j2c +src/valid-16x16.j2c : 16 x 16, 3 channel, uint8 jpeg2000 + SHA-1: 82BF87BEB48AFCB8E475AE35F065A15B2ED05F00 + channel list: R, G, B + oiio:BitsPerSample: 8 + oiio:ColorSpace: "srgb_rec709_scene" diff --git a/testsuite/htj2k/run.py b/testsuite/htj2k/run.py index 4d34433685..9356a81bae 100644 --- a/testsuite/htj2k/run.py +++ b/testsuite/htj2k/run.py @@ -8,6 +8,9 @@ # These tests are checking the openjph library that can optionally be compiled into the Jpeg2000 # plugin of OIIO. If the library is not enabled, these will fail. +# Capture stderr too, so the rejection messages below are checked. +redirect = ' >> out.txt 2>&1 ' + command += oiiotool(OIIO_TESTSUITE_IMAGEDIR+"/tahoe-gps.jpg" " -o test.j2c") @@ -23,3 +26,11 @@ " --attrib qstep 0.03 -o testcompress.j2c") command += diff_command(OIIO_TESTSUITE_IMAGEDIR+"/tahoe-gps.jpg", "testcompress.j2c", extraargs="-fail 0.11") + +# Malformed HTJ2K headers, built by src/make_malformed_htj2k.py. The HTJ2K +# reader allocates the whole image up front from the SIZ dimensions, so an +# oversized claim has to be rejected at open time rather than at allocation. +command += oiiotool("--info -v src/bomb-resolution.j2c", failureok = True) +command += oiiotool("--info -v src/bomb-ratio.j2c", failureok = True) +# ... while a small valid codestream still reads. +command += oiiotool("--info -v --hash src/valid-16x16.j2c", failureok = True) diff --git a/testsuite/htj2k/src/bomb-ratio.j2c b/testsuite/htj2k/src/bomb-ratio.j2c new file mode 100644 index 0000000000000000000000000000000000000000..93a8a80e33603f804d7bec318ecd16985a0a38f1 GIT binary patch literal 310 zcmezG|38pH-+_UFv26huGe7}4pP8MJ5rX~)FmNz10hKZT4`Sc}vKSdzSy&tx|Hm*0 zD+EL!fq;O3|0xXO42=E-sd-)j9tvTpMG6Lb#s+$ZdjBUdaDlDb&A|A7@&TcSiwx~0 z5W5d>Ca`Q^3J_of3V?}jCU%h20j>_lKn5*_-lQ{z42Mk^y3ZtmY2L0bcMu<>;s;}f z1J?%Gd^yWmHZ^}(FCJ{VRL&rKTPDyZK+f(T(>V@@Hn z2s8YxtM6aG|NcHkKas_aOqC3O|NfJkfB*e^1`dbnZ-$I>b~F6_d_W--sN?@l0GA|L A(*OVf literal 0 HcmV?d00001 diff --git a/testsuite/htj2k/src/bomb-resolution.j2c b/testsuite/htj2k/src/bomb-resolution.j2c new file mode 100644 index 0000000000000000000000000000000000000000..2793477975dfe2d5bc7755a53bdcae6a9fa6fcec GIT binary patch literal 310 zcmezG|38pH-+`grbP5_lKn5*_-lQ{z42Mk^y3ZtmY2L0bcMu<>;s;}f z1J?%Gd^yWmHZ^}(FCJ{VRL&rKTPDyZK+f(T(>V@@Hn z2s8YxtM6aG|NcHkKas_aOqC3O|NfJkfB*e^1`dbnZ-$I>b~F6_d_W--sN?@l01ty& AHvj+t literal 0 HcmV?d00001 diff --git a/testsuite/htj2k/src/make_malformed_htj2k.py b/testsuite/htj2k/src/make_malformed_htj2k.py new file mode 100644 index 0000000000..7fb48d3c9f --- /dev/null +++ b/testsuite/htj2k/src/make_malformed_htj2k.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 + +# Copyright Contributors to the OpenImageIO project. +# SPDX-License-Identifier: Apache-2.0 +# https://github.com/AcademySoftwareFoundation/OpenImageIO + +"""Generator for the malformed HTJ2K fixtures in this directory. + +The files it writes are committed, so this only needs to be run if they must +be regenerated: + + python3 make_malformed_htj2k.py . + +It patches the SIZ marker segment of the committed valid-16x16.j2c, so it +needs no J2K encoder. valid-16x16.j2c itself was produced with: + + oiiotool --pattern checker 16x16 3 -d uint8 -o valid-16x16.j2c + +A raw J2K codestream starts with SOC (0xFF4F) followed by SIZ (0xFF51). Within +the SIZ segment, counting from the marker itself, Xsiz/Ysiz are at +6/+10 and +the tile size XTsiz/YTsiz at +22/+26. Those four fields are all the reader +consults for the image dimensions, so patching them is enough to make a tiny +file claim an enormous image. +""" + +import struct +import sys + +SIZ_XSIZ = 6 +SIZ_XTSIZ = 22 + + +def patch_size(base, width, height): + d = bytearray(base) + i = d.find(b'\xff\x51') + if i < 0: + raise RuntimeError('no SIZ marker found') + struct.pack_into('>II', d, i + SIZ_XSIZ, width, height) + struct.pack_into('>II', d, i + SIZ_XTSIZ, width, height) + return bytes(d) + + +def main(outdir): + with open(outdir + '/valid-16x16.j2c', 'rb') as f: + base = f.read() + + def write(name, data): + with open(outdir + '/' + name, 'wb') as f: + f.write(data) + print('wrote %s (%d bytes)' % (name, len(data))) + + # Past the per-dimension ceiling ("limits:resolution"). Before the guards + # were added this reached m_buf.resize() and aborted the process with an + # uncaught std::bad_alloc. + write('bomb-resolution.j2c', patch_size(base, 2000000000, 2000000000)) + + # Under both the per-dimension ceiling and "limits:imagesize_MB" (30 GB), + # so only the declared-size-to-file-size ratio can reject this one. + write('bomb-ratio.j2c', patch_size(base, 100000, 100000)) + + +if __name__ == '__main__': + main(sys.argv[1] if len(sys.argv) > 1 else '.') diff --git a/testsuite/htj2k/src/valid-16x16.j2c b/testsuite/htj2k/src/valid-16x16.j2c new file mode 100644 index 0000000000000000000000000000000000000000..d5712165be0b783fd8e8a5953d9520757f0210c1 GIT binary patch literal 310 zcmezG|38pH-+=)L1fUqi1_4w)Gdm+A1pNnY7Y{aFDrbptfB(tNzyJO{1BXNPH$%obyBYp|KA;c^)bal&0I1zn AcK`qY literal 0 HcmV?d00001