merges common fixes and move bounds check to central location

PR AcademySoftwareFoundation#401 had conflicts, and some of the checks were not in a central
location. This incorporates those changes, moving the extra range checks
to the central sanityCheck already in ImfHeader. Then adds a new utility
function for computing the pointer offsets that can prevent simple
overflow when there are large offsets from origin or widths with
subsampling.

Signed-off-by: Kimball Thurston <[email protected]>
Co-Authored-By: pgajdos <[email protected]>

OpenEXR/IlmImf/ImfFrameBuffer.h

@@ -48,13 +48,37 @@
 #include "ImfPixelType.h"
 #include "ImfExport.h"
 #include "ImfNamespace.h"
+#include "ImathBox.h"
 
 #include <map>
 #include <string>
 
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER
 
+//-------------------------------------------------------
+// Utility to compute the origin-based pointer address
+//
+// With large offsets for the data window, the naive code
+// can wrap around, especially on 32-bit machines.
+// This can be used to avoid that
+//-------------------------------------------------------
+
+template <typename T>
+inline T *
+ComputeOriginPointer( T *p, const IMATH_NAMESPACE::Box2i &dw, int xSamp = 1, int ySamp = 1 )
+{
+    // data window is an int, so force promote to higher type to avoid
+    // overflow for off y (degenerate size checks should be in
+    // ImfHeader::sanityCheck, but offset can be large-ish)
+    long long basex = static_cast<long long>( dw.min.x );
+    long long w = static_cast<long long>( dw.max.x ) - basex + 1;
+    long long offy = ( static_cast<long long>( dw.min.y ) /
+                       static_cast<long long>( ySamp ) );
+    offy *= w / static_cast<long long>( xSamp );
+
+    return p - offy - ( basex / static_cast<long long>( xSamp ) );
+}
 
 //-------------------------------------------------------
 // Description of a single slice of the frame buffer:

OpenEXR/IlmImf/ImfHeader.cpp

@@ -785,30 +785,46 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
 	throw IEX_NAMESPACE::ArgExc ("Invalid data window in image header.");
     }
 
-    if (maxImageWidth > 0 &&
-        maxImageWidth < (dataWindow.max.x - dataWindow.min.x + 1))
+    int w = (dataWindow.max.x - dataWindow.min.x + 1);
+    if (maxImageWidth > 0 && maxImageWidth < w)
     {
 	THROW (IEX_NAMESPACE::ArgExc, "The width of the data window exceeds the "
 			    "maximum width of " << maxImageWidth << "pixels.");
     }
 
-    if (maxImageHeight > 0 &&
-	maxImageHeight < dataWindow.max.y - dataWindow.min.y + 1)
+    int h = (dataWindow.max.y - dataWindow.min.y + 1);
+    if (maxImageHeight > 0 && maxImageHeight < h)
     {
-	THROW (IEX_NAMESPACE::ArgExc, "The width of the data window exceeds the "
-			    "maximum width of " << maxImageHeight << "pixels.");
+	THROW (IEX_NAMESPACE::ArgExc, "The height of the data window exceeds the "
+			    "maximum height of " << maxImageHeight << "pixels.");
+    }
+
+    // make sure to avoid simple math overflow for large offsets
+    // we know we're at a positive width because of checks above
+    long long bigW = static_cast<long long>( w );
+    long long absOffY = std::abs ( dataWindow.min.y );
+    long long absOffX = std::abs ( dataWindow.min.x );
+    long long offX = static_cast<long long>( INT_MAX ) - absOffX;
+    long long offsetCount = absOffY * bigW;
+    long long bytesLeftPerLine = static_cast<long long>( INT_MAX ) / bigW;
+    if (bytesLeftPerLine < absOffY || offX < offsetCount)
+    {
+	THROW (IEX_NAMESPACE::ArgExc, "Data window [ (" << dataWindow.min.x
+	       << ", " << dataWindow.min.x << ") - (" << dataWindow.max.x
+	       << ", " << dataWindow.max.x
+	       << ") ] offset / size will overflow pointer calculations");
     }
 
-   // chunk table must be smaller than the maximum image area
-   // (only reachable for unknown types or damaged files: will have thrown earlier
-   //  for regular image types)
-   if( maxImageHeight>0 && maxImageWidth>0 && 
-       hasChunkCount() && chunkCount()>Int64(maxImageWidth)*Int64(maxImageHeight))
-   {
-       THROW (IEX_NAMESPACE::ArgExc, "chunkCount exceeds maximum area of "
-       << Int64(maxImageWidth)*Int64(maxImageHeight) << " pixels." );
+    // chunk table must be smaller than the maximum image area
+    // (only reachable for unknown types or damaged files: will have thrown earlier
+    //  for regular image types)
+    if( maxImageHeight>0 && maxImageWidth>0 && 
+	hasChunkCount() && chunkCount()>Int64(maxImageWidth)*Int64(maxImageHeight))
+    {
+	THROW (IEX_NAMESPACE::ArgExc, "chunkCount exceeds maximum area of "
+	       << Int64(maxImageWidth)*Int64(maxImageHeight) << " pixels." );
 
-   }
+    }
 
 
     //

OpenEXR/exr2aces/main.cpp

@@ -124,7 +124,7 @@ exr2aces (const char inFileName[],
 	height = dw.max.y - dw.min.y + 1;
 	p.resizeErase (height, width);
 
-	in.setFrameBuffer (&p[0][0] - intptr_t( dw.min.x ) - intptr_t( dw.min.y ) * intptr_t( width ), 1, width);
+	in.setFrameBuffer (ComputeOriginPointer (&p[0][0], dw), 1, width);
 	in.readPixels (dw.min.y, dw.max.y);
     }
 
@@ -147,7 +147,7 @@ exr2aces (const char inFileName[],
 
 	AcesOutputFile out (outFileName, h, ch);
 
-	out.setFrameBuffer (&p[0][0] - intptr_t( dw.min.x ) - intptr_t( dw.min.y ) * intptr_t( width ), 1, width);
+	out.setFrameBuffer (ComputeOriginPointer (&p[0][0], dw), 1, width);
 	out.writePixels (height);
     }
 }

OpenEXR/exrenvmap/readInputImage.cpp

@@ -194,7 +194,7 @@ readSixImages (const char inFileName[],
                    "from the data window of other cube faces.");
         }
 
-        in.setFrameBuffer (pixels - intptr_t( dw.min.x ) - intptr_t( dw.min.y ) * intptr_t( w ), 1, w);
+        in.setFrameBuffer (ComputeOriginPointer (pixels, dw), 1, w);
         in.readPixels (dw.min.y, dw.max.y);
 
         pixels += w * h;

OpenEXR/exrmakepreview/makePreview.cpp

@@ -110,7 +110,7 @@ generatePreview (const char inFileName[],
     int h = dw.max.y - dw.min.y + 1;
 
     Array2D <Rgba> pixels (h, w);
-    in.setFrameBuffer (&pixels[0][0] - intptr_t( dw.min.y ) * intptr_t( w ) - intptr_t( dw.min.x ), 1, w);
+    in.setFrameBuffer (ComputeOriginPointer (&pixels[0][0], dw), 1, w);
     in.readPixels (dw.min.y, dw.max.y);
 
     //

OpenEXR/exrmaketiled/Image.h

@@ -190,12 +190,11 @@ OPENEXR_IMF_INTERNAL_NAMESPACE::Slice
 TypedImageChannel<T>::slice () const
 {
     const IMATH_NAMESPACE::Box2i &dw = image().dataWindow();
-    int w = dw.max.x - dw.min.x + 1;
 
-    return OPENEXR_IMF_INTERNAL_NAMESPACE::Slice (pixelType(),
-		       (char *) (&_pixels[0][0] - dw.min.y * w - dw.min.x),
-		       sizeof (T),
-		       w * sizeof (T));
+    return OPENEXR_IMF_INTERNAL_NAMESPACE::Slice (
+        pixelType(),
+        (char *) ComputeOriginPointer (&_pixels[0][0], dw),
+        sizeof (T), w * sizeof (T));
 }
 
 

OpenEXR/exrmultiview/Image.h

@@ -159,6 +159,8 @@ TypedImageChannel<T>::TypedImageChannel
     _ySampling (ySampling),
     _pixels (0, 0)
 {
+    if ( _xSampling < 1 || _ySampling < 1 )
+        throw IEX_NAMESPACE::ArgExc ("Invalid x/y sampling values");
     resize();
 }
 
@@ -202,9 +204,8 @@ TypedImageChannel<T>::slice () const
     int w = dw.max.x - dw.min.x + 1;
 
     return IMF::Slice (pixelType(),
-		       (char *) (&_pixels[0][0] -
-				 dw.min.y / _ySampling * (w / _xSampling) -
-				 dw.min.x / _xSampling),
+                       (char *) IMF::ComputeOriginPointer(
+                           &_pixels[0][0], dw, _xSampling, _ySampling ),
 		       sizeof (T),
 		       (w / _xSampling) * sizeof (T),
 		       _xSampling,
@@ -227,7 +228,9 @@ template <class T>
 void
 TypedImageChannel<T>::black ()
 {
-    memset(&_pixels[0][0],0,image().width()/_xSampling*image().height()/_ySampling*sizeof(T));
+    size_t nx = static_cast<size_t>( image().width() ) / static_cast<size_t>( _xSampling );
+    size_t ny = static_cast<size_t>( image().height() ) / static_cast<size_t>( _ySampling );
+    memset(&_pixels[0][0],0,nx*ny*sizeof(T));
 }