fix(jpeg2000): guard the HTJ2K reader, not just the OpenJPEG one

src/jpeg2000.imageio/jpeg2000input.cpp

@@ -338,23 +338,28 @@ 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);
 
     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<int>::max(), 0,
+                              std::numeric_limits<int>::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
 

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"

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)

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 '.')