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 0000000000..93a8a80e33 Binary files /dev/null and b/testsuite/htj2k/src/bomb-ratio.j2c differ diff --git a/testsuite/htj2k/src/bomb-resolution.j2c b/testsuite/htj2k/src/bomb-resolution.j2c new file mode 100644 index 0000000000..2793477975 Binary files /dev/null and b/testsuite/htj2k/src/bomb-resolution.j2c differ 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 0000000000..d5712165be Binary files /dev/null and b/testsuite/htj2k/src/valid-16x16.j2c differ