Fix out-of-bounds reads when using OpenEXR decreasingY lineOrder.

src/cmake/testing.cmake

@@ -278,7 +278,7 @@ macro (oiio_add_all_tests)
                     IMAGEDIR j2kp4files_v1_5
                     URL http://www.itu.int/net/ITU-T/sigdb/speimage/ImageForm-s.aspx?val=10100803)
     set (all_openexr_tests
-         openexr-suite openexr-multires openexr-chroma
+         openexr-suite openexr-multires openexr-chroma openexr-decreasingy
          openexr-v2 openexr-window perchannel oiiotool-deep)
     if (USE_PYTHON AND NOT SANITIZE)
         list (APPEND all_openexr_tests openexr-copy)

src/libOpenImageIO/imagebuf.cpp

@@ -1418,8 +1418,20 @@ ImageBuf::write(ImageOutput* out, ProgressCallback progress_callback,
             int chunk = clamp(round_to_multiple(int(budget / slsize), 64), 1,
                               1024);
             std::unique_ptr<char[]> tmp(new char[chunk * slsize]);
+
+            const bool isDecreasingY = !strcmp(out->format_name(), "openexr")
+                                       && outspec.get_string_attribute(
+                                              "openexr:lineOrder")
+                                              == "decreasingY";
+            const int yStart = isDecreasingY
+                                   ? ((outspec.height - 1) / chunk) * chunk
+                                   : 0;
+            const int yDelta = isDecreasingY ? -chunk : chunk;
             for (int z = 0; z < outspec.depth; ++z) {
-                for (int y = 0; y < outspec.height && ok; y += chunk) {
+                for (int y = yStart; ((isDecreasingY && y >= 0)
+                                      || (!isDecreasingY && y < outspec.height))
+                                     && ok;
+                     y += yDelta) {
                     int yend = std::min(y + outspec.y + chunk,
                                         outspec.y + outspec.height);
                     ok &= get_pixels(ROI(outspec.x, outspec.x + outspec.width,
@@ -1430,10 +1442,11 @@ ImageBuf::write(ImageOutput* out, ProgressCallback progress_callback,
                                                z + outspec.z, bufformat,
                                                &tmp[0]);
                     if (progress_callback
-                        && progress_callback(progress_callback_data,
-                                             (float)(z * outspec.height + y)
-                                                 / (outspec.height
-                                                    * outspec.depth)))
+                        && progress_callback(
+                            progress_callback_data,
+                            (float)(z * outspec.height
+                                    + (isDecreasingY ? (outspec.height - y) : y))
+                                / (outspec.height * outspec.depth)))
                         return ok;
                 }
             }

src/libOpenImageIO/imageoutput.cpp

@@ -518,17 +518,31 @@ ImageOutput::write_image(TypeDesc format, const void* data, stride_t xstride,
         int rps   = m_spec.get_int_attribute("tiff:RowsPerStrip", 64);
         int chunk = std::max(1, (1 << 26) / int(m_spec.scanline_bytes(true)));
         chunk     = round_to_multiple(chunk, rps);
+
+        const bool isDecreasingY = !strcmp(format_name(), "openexr")
+                                   && m_spec.get_string_attribute(
+                                          "openexr:lineOrder")
+                                          == "decreasingY";
+        const int yStart = isDecreasingY ? ((m_spec.height - 1) / chunk) * chunk
+                                         : 0;
+        const int yDelta = isDecreasingY ? -chunk : chunk;
+
         for (int z = 0; z < m_spec.depth; ++z)
-            for (int y = 0; y < m_spec.height && ok; y += chunk) {
+            for (int y = yStart; ((isDecreasingY && y >= 0)
+                                  || (!isDecreasingY && y < m_spec.height))
+                                 && ok;
+                 y += yDelta) {
                 int yend      = std::min(y + m_spec.y + chunk,
                                          m_spec.y + m_spec.height);
                 const char* d = (const char*)data + z * zstride + y * ystride;
                 ok &= write_scanlines(y + m_spec.y, yend, z + m_spec.z, format,
                                       d, xstride, ystride);
                 if (progress_callback
-                    && progress_callback(progress_callback_data,
-                                         (float)(z * m_spec.height + y)
-                                             / (m_spec.height * m_spec.depth)))
+                    && progress_callback(
+                        progress_callback_data,
+                        (float)(z * m_spec.height
+                                + (isDecreasingY ? (m_spec.height - y) : y))
+                            / (m_spec.height * m_spec.depth)))
                     return ok;
             }
     }

src/openexr.imageio/exroutput.cpp

@@ -1485,21 +1485,34 @@ OpenEXROutput::write_scanlines(int ybegin, int yend, int z, TypeDesc format,
                               * 1024;  // Allocate 16 MB, or 1 scanline
     int chunk = std::max(1, int(limit / scanlinebytes));
 
-    bool ok = true;
-    for (; ok && ybegin < yend; ybegin += chunk) {
-        int y1         = std::min(ybegin + chunk, yend);
-        int nscanlines = y1 - ybegin;
-        const void* d  = to_native_rectangle(m_spec.x, m_spec.x + m_spec.width,
-                                             ybegin, y1, z, z + 1, format, data,
-                                             xstride, ystride, zstride,
-                                             m_scratch);
+    bool ok         = true;
+    const bool isDecreasingY = m_spec.get_string_attribute("openexr:lineOrder")
+                               == "decreasingY";
+
+    int yChunkStart = isDecreasingY ? ybegin + ((yend - ybegin) / chunk) * chunk
+                                    : ybegin;
+    const int yDelta = isDecreasingY ? -chunk : chunk;
+    for (; ok
+           && ((isDecreasingY && yChunkStart >= ybegin)
+               || (!isDecreasingY && yChunkStart < yend));
+         yChunkStart += yDelta) {
+        int y1         = std::min(yChunkStart + chunk, yend);
+        int nscanlines = y1 - yChunkStart;
+
+        const void* dataStart = (const char*)data
+                                + (yChunkStart - ybegin) * ystride;
+        const void* d = to_native_rectangle(m_spec.x, m_spec.x + m_spec.width,
+                                            yChunkStart, y1, z, z + 1, format,
+                                            dataStart, xstride, ystride,
+                                            zstride, m_scratch);
 
         // Compute where OpenEXR needs to think the full buffers starts.
         // OpenImageIO requires that 'data' points to where client stored
         // the bytes to be written, but OpenEXR's frameBuffer.insert() wants
         // where the address of the "virtual framebuffer" for the whole
         // image.
-        char* buf = (char*)d - m_spec.x * pixel_bytes - ybegin * scanlinebytes;
+        char* buf = (char*)d - m_spec.x * pixel_bytes
+                    - yChunkStart * scanlinebytes;
         try {
             Imf::FrameBuffer frameBuffer;
             size_t chanoffset = 0;
@@ -1527,8 +1540,6 @@ OpenEXROutput::write_scanlines(int ybegin, int yend, int z, TypeDesc format,
             errorf("Failed OpenEXR write: unknown exception");
             return false;
         }
-
-        data = (const char*)data + ystride * nscanlines;
     }
 
     // If we allocated more than 1M, free the memory.  It's not wasteful,

testsuite/openexr-decreasingy/ref/out.txt

@@ -0,0 +1,74 @@
+Reading ref/decreasingY-resize.exr
+ref/decreasingY-resize.exr : 4080 x 3072, 3 channel, half openexr
+    SHA-1: 34A9F9879CD89E718ABCEE718A779035F6F78666
+    channel list: R, G, B
+    compression: "zip"
+    DateTime: "2013:04:16 10:20:35"
+    Orientation: 1 (normal)
+    PixelAspectRatio: 1
+    ResolutionUnit: "in"
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    Software: "OpenImageIO 2.6.2.0dev : oiiotool ../common/tahoe-tiny.tif --resize 4080x3072 -o ref/decreasingY-resize.exr"
+    XResolution: 72
+    YResolution: 72
+    Exif:ImageHistory: "oiiotool ../common/tahoe-tiny.tif --resize 4080x3072 -o ref/decreasingY-resize.exr"
+    oiio:ColorSpace: "Linear"
+    oiio:subimages: 1
+Reading ref/decreasingY-copy.exr
+ref/decreasingY-copy.exr : 4080 x 3072, 3 channel, half openexr
+    SHA-1: 34A9F9879CD89E718ABCEE718A779035F6F78666
+    channel list: R, G, B
+    compression: "zip"
+    DateTime: "2013:04:16 10:20:35"
+    Orientation: 1 (normal)
+    PixelAspectRatio: 1
+    ResolutionUnit: "in"
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    Software: "OpenImageIO 2.6.2.0dev : oiiotool ref/decreasingY-resize.exr -o ref/decreasingY-copy.exr"
+    XResolution: 72
+    YResolution: 72
+    Exif:ImageHistory: "oiiotool ../common/tahoe-tiny.tif --resize 4080x3072 -o ref/decreasingY-resize.exr
+oiiotool ref/decreasingY-resize.exr -o ref/decreasingY-copy.exr"
+    oiio:ColorSpace: "Linear"
+    oiio:subimages: 1
+Reading decreasingY-resize.exr
+decreasingY-resize.exr : 4080 x 3072, 3 channel, half openexr
+    SHA-1: 34A9F9879CD89E718ABCEE718A779035F6F78666
+    channel list: R, G, B
+    compression: "zip"
+    DateTime: "2013:04:16 10:20:35"
+    Orientation: 1 (normal)
+    PixelAspectRatio: 1
+    ResolutionUnit: "in"
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    Software: "OpenImageIO 2.6.2.0dev : oiiotool ../common/tahoe-tiny.tif --resize 4080x3072 --attrib openexr:lineOrder decreasingY -o decreasingY-resize.exr"
+    XResolution: 72
+    YResolution: 72
+    Exif:ImageHistory: "oiiotool ../common/tahoe-tiny.tif --resize 4080x3072 --attrib openexr:lineOrder decreasingY -o decreasingY-resize.exr"
+    oiio:ColorSpace: "Linear"
+    oiio:subimages: 1
+Reading decreasingY-copy.exr
+decreasingY-copy.exr : 4080 x 3072, 3 channel, half openexr
+    SHA-1: 34A9F9879CD89E718ABCEE718A779035F6F78666
+    channel list: R, G, B
+    compression: "zip"
+    DateTime: "2013:04:16 10:20:35"
+    Orientation: 1 (normal)
+    PixelAspectRatio: 1
+    ResolutionUnit: "in"
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    Software: "OpenImageIO 2.6.2.0dev : oiiotool ref/decreasingY-resize.exr --attrib openexr:lineOrder decreasingY -o decreasingY-copy.exr"
+    XResolution: 72
+    YResolution: 72
+    Exif:ImageHistory: "oiiotool ../common/tahoe-tiny.tif --resize 4080x3072 -o ref/decreasingY-resize.exr
+oiiotool ref/decreasingY-resize.exr --attrib openexr:lineOrder decreasingY -o decreasingY-copy.exr"
+    oiio:ColorSpace: "Linear"
+    oiio:subimages: 1
+Comparing "decreasingY-resize.exr" and "ref/decreasingY-resize.exr"
+PASS
+Comparing "decreasingY-copy.exr" and "ref/decreasingY-copy.exr"
+PASS

testsuite/openexr-decreasingy/run.py

@@ -0,0 +1,41 @@
+#!/usr/bin/env python
+
+# Copyright Contributors to the OpenImageIO project.
+# SPDX-License-Identifier: Apache-2.0
+# https://github.com/AcademySoftwareFoundation/OpenImageIO
+
+import os, sys
+
+####################################################################
+# Verify decreasingY line order generates same image as increasingY (default).
+####################################################################
+
+# Capture error output
+redirect = " >> out.txt 2>&1 "
+
+# Create reference images stored in increasingY order (default)
+# Resizing to a large image size to ensure scanline chunking logic is triggered.
+command += oiiotool("../common/tahoe-tiny.tif --resize 4080x3072 -o ref/decreasingY-resize.exr")
+command += info_command("ref/decreasingY-resize.exr")
+command += oiiotool("ref/decreasingY-resize.exr -o ref/decreasingY-copy.exr")
+command += info_command("ref/decreasingY-copy.exr")
+
+# Create an image in decreasing order via resizing (Tests ImageOutput::write_image() logic)
+command += oiiotool("../common/tahoe-tiny.tif --resize 4080x3072 --attrib openexr:lineOrder decreasingY -o decreasingY-resize.exr")
+command += info_command("decreasingY-resize.exr")
+
+# Create an image in decreasing order via copying a reference image. (Tests ImageBuf::write() logic)
+command += oiiotool("ref/decreasingY-resize.exr --attrib openexr:lineOrder decreasingY -o decreasingY-copy.exr")
+command += info_command("decreasingY-copy.exr")
+
+# Outputs to check against references.
+# This makes sure the images look the same since the line order is a storage detail and should not
+# change how the image actually looks. These comparisons help verify chunk order and scanlines are
+# processed properly.
+outputs = [
+    "decreasingY-resize.exr",
+    "decreasingY-copy.exr",
+]
+
+
+#print "Running this command:\n" + command + "\n"