Backports of From ec64836c2312b13034149acab499c112bd289cd9 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Sun, 21 Jul 2019 17:56:30 +1200 Subject: [PATCH] Refactor origin function to a Slice factory and Rgba custom utility From a7eec54765e9122b78a6c34bb9d5bf744631bea2 Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Mon, 1 Jul 2019 22:11:34 +1200 Subject: [PATCH] merges common fixes and move bounds check to central location From 45f9912e6cfa0617ec2054d96d1e1e73fad4a62a Mon Sep 17 00:00:00 2001 From: Kimball Thurston Date: Thu, 27 Jun 2019 23:36:09 +1200 Subject: [PATCH] Fix part of #232, issue with pointer overflows --- openexr-2.2.1.orig/IlmImf/ImfFrameBuffer.cpp +++ openexr-2.2.1/IlmImf/ImfFrameBuffer.cpp @@ -74,6 +74,88 @@ Slice::Slice (PixelType t, // empty } +Slice +Slice::Make ( + PixelType type, + const void* ptr, + const IMATH_NAMESPACE::V2i& origin, + int64_t w, + int64_t h, + size_t xStride, + size_t yStride, + int xSampling, + int ySampling, + double fillValue, + bool xTileCoords, + bool yTileCoords) +{ + char* base = reinterpret_cast (const_cast (ptr)); + if (xStride == 0) + { + switch (type) + { + case UINT: xStride = sizeof (uint32_t); break; + case HALF: xStride = sizeof (uint16_t); break; + case FLOAT: xStride = sizeof (float); break; + case NUM_PIXELTYPES: + THROW (IEX_NAMESPACE::ArgExc, "Invalid pixel type."); + } + } + if (yStride == 0) + yStride = static_cast (w / xSampling) * xStride; + + // 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) + int64_t offx = (static_cast (origin.x) / + static_cast (xSampling)); + offx *= static_cast (xStride); + + int64_t offy = (static_cast (origin.y) / + static_cast (ySampling)); + offy *= static_cast (yStride); + + return Slice ( + type, + base - offx - offy, + xStride, + yStride, + xSampling, + ySampling, + fillValue, + xTileCoords, + yTileCoords); +} + +Slice +Slice::Make ( + PixelType type, + const void* ptr, + const IMATH_NAMESPACE::Box2i& dataWindow, + size_t xStride, + size_t yStride, + int xSampling, + int ySampling, + double fillValue, + bool xTileCoords, + bool yTileCoords) +{ + return Make ( + type, + ptr, + dataWindow.min, + static_cast (dataWindow.max.x) - + static_cast (dataWindow.min.x) + 1, + static_cast (dataWindow.max.y) - + static_cast (dataWindow.min.y) + 1, + xStride, + yStride, + xSampling, + ySampling, + fillValue, + xTileCoords, + yTileCoords); +} void FrameBuffer::insert (const char name[], const Slice &slice) --- openexr-2.2.1.orig/IlmImf/ImfFrameBuffer.h +++ openexr-2.2.1/IlmImf/ImfFrameBuffer.h @@ -48,14 +48,15 @@ #include "ImfPixelType.h" #include "ImfExport.h" #include "ImfNamespace.h" +#include "ImathBox.h" #include #include +#include OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER - //------------------------------------------------------- // Description of a single slice of the frame buffer: // @@ -147,6 +148,36 @@ struct IMF_EXPORT Slice double fillValue = 0.0, bool xTileCoords = false, bool yTileCoords = false); + + // Does the heavy lifting of computing the base pointer for a slice, + // avoiding overflow issues with large origin offsets + // + // if xStride == 0, assumes sizeof(pixeltype) + // if yStride == 0, assumes xStride * ( w / xSampling ) + static Slice Make(PixelType type, + const void *ptr, + const IMATH_NAMESPACE::V2i &origin, + int64_t w, + int64_t h, + size_t xStride = 0, + size_t yStride = 0, + int xSampling = 1, + int ySampling = 1, + double fillValue = 0.0, + bool xTileCoords = false, + bool yTileCoords = false); + // same as above, just computes w and h for you + // from a data window + static Slice Make(PixelType type, + const void *ptr, + const IMATH_NAMESPACE::Box2i &dataWindow, + size_t xStride = 0, + size_t yStride = 0, + int xSampling = 1, + int ySampling = 1, + double fillValue = 0.0, + bool xTileCoords = false, + bool yTileCoords = false); }; --- openexr-2.2.1.orig/IlmImf/ImfHeader.cpp +++ openexr-2.2.1/IlmImf/ImfHeader.cpp @@ -785,30 +785,46 @@ Header::sanityCheck (bool isTiled, bool 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( w ); + long long absOffY = std::abs ( dataWindow.min.y ); + long long absOffX = std::abs ( dataWindow.min.x ); + long long offX = static_cast( INT_MAX ) - absOffX; + long long offsetCount = absOffY * bigW; + long long bytesLeftPerLine = static_cast( 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-2.2.1.orig/IlmImf/ImfRgbaFile.h +++ openexr-2.2.1/IlmImf/ImfRgbaFile.h @@ -60,6 +60,65 @@ 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 +//------------------------------------------------------- + +inline const Rgba * +ComputeBasePointer ( + const Rgba* ptr, + const IMATH_NAMESPACE::V2i& origin, + int64_t w, + size_t xStride = 1, + size_t yStride = 0) +{ + if (yStride == 0) + yStride = w; + int64_t offx = static_cast (origin.x); + offx *= xStride; + int64_t offy = static_cast (origin.y); + offy *= yStride; + return ptr - offx - offy; +} + +inline const Rgba * +ComputeBasePointer (const Rgba* ptr, const IMATH_NAMESPACE::Box2i& dataWindow) +{ + return ComputeBasePointer (ptr, dataWindow.min, + static_cast (dataWindow.max.x) - + static_cast (dataWindow.min.x) + 1); +} + +inline Rgba* +ComputeBasePointer ( + Rgba* ptr, + const IMATH_NAMESPACE::V2i& origin, + int64_t w, + size_t xStride = 1, + size_t yStride = 0) +{ + if (yStride == 0) + yStride = w; + int64_t offx = static_cast (origin.x); + offx *= xStride; + int64_t offy = static_cast (origin.y); + offy *= yStride; + return ptr - offx - offy; +} + +inline Rgba* +ComputeBasePointer (Rgba* ptr, const IMATH_NAMESPACE::Box2i& dataWindow) +{ + return ComputeBasePointer ( + ptr, + dataWindow.min, + static_cast (dataWindow.max.x) - + static_cast (dataWindow.min.x) + 1); +} // // RGBA output file. --- openexr-2.2.1.orig/IlmImf/ImfScanLineInputFile.cpp +++ openexr-2.2.1/IlmImf/ImfScanLineInputFile.cpp @@ -522,14 +522,14 @@ LineBufferTask::execute () if (_lineBuffer->uncompressedData == 0) { - int uncompressedSize = 0; + size_t uncompressedSize = 0; int maxY = min (_lineBuffer->maxY, _ifd->maxY); for (int i = _lineBuffer->minY - _ifd->minY; i <= maxY - _ifd->minY; ++i) { - uncompressedSize += (int) _ifd->bytesPerLine[i]; + uncompressedSize += _ifd->bytesPerLine[i]; } if (_lineBuffer->compressor && @@ -626,11 +626,11 @@ LineBufferTask::execute () // char *linePtr = slice.base + - divp (y, slice.ySampling) * - slice.yStride; + intptr_t( divp (y, slice.ySampling) ) * + intptr_t( slice.yStride ); - char *writePtr = linePtr + dMinX * slice.xStride; - char *endPtr = linePtr + dMaxX * slice.xStride; + char *writePtr = linePtr + intptr_t( dMinX ) * intptr_t( slice.xStride ); + char *endPtr = linePtr + intptr_t( dMaxX ) * intptr_t( slice.xStride ); copyIntoFrameBuffer (readPtr, writePtr, endPtr, slice.xStride, slice.fill, @@ -836,14 +836,14 @@ LineBufferTaskIIF::execute() if (_lineBuffer->uncompressedData == 0) { - int uncompressedSize = 0; + size_t uncompressedSize = 0; int maxY = min (_lineBuffer->maxY, _ifd->maxY); for (int i = _lineBuffer->minY - _ifd->minY; i <= maxY - _ifd->minY; ++i) { - uncompressedSize += (int) _ifd->bytesPerLine[i]; + uncompressedSize += _ifd->bytesPerLine[i]; } if (_lineBuffer->compressor && --- openexr-2.2.1.orig/exrenvmap/readInputImage.cpp +++ openexr-2.2.1/exrenvmap/readInputImage.cpp @@ -194,7 +194,7 @@ readSixImages (const char inFileName[], "from the data window of other cube faces."); } - in.setFrameBuffer (pixels - dw.min.x - dw.min.y * w, 1, w); + in.setFrameBuffer (ComputeBasePointer (pixels, dw), 1, w); in.readPixels (dw.min.y, dw.max.y); pixels += w * h; --- openexr-2.2.1.orig/exrmakepreview/makePreview.cpp +++ openexr-2.2.1/exrmakepreview/makePreview.cpp @@ -110,7 +110,7 @@ generatePreview (const char inFileName[] int h = dw.max.y - dw.min.y + 1; Array2D pixels (h, w); - in.setFrameBuffer (&pixels[0][0] - dw.min.y * w - dw.min.x, 1, w); + in.setFrameBuffer (ComputeBasePointer (&pixels[0][0], dw), 1, w); in.readPixels (dw.min.y, dw.max.y); // --- openexr-2.2.1.orig/exrmaketiled/Image.h +++ openexr-2.2.1/exrmaketiled/Image.h @@ -190,12 +190,12 @@ OPENEXR_IMF_INTERNAL_NAMESPACE::Slice TypedImageChannel::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::Make ( + pixelType(), + &_pixels[0][0], + dw, + sizeof (T)); } --- openexr-2.2.1.orig/exrmultiview/Image.h +++ openexr-2.2.1/exrmultiview/Image.h @@ -159,6 +159,8 @@ TypedImageChannel::TypedImageChannel _ySampling (ySampling), _pixels (0, 0) { + if ( _xSampling < 1 || _ySampling < 1 ) + throw IEX_NAMESPACE::ArgExc ("Invalid x/y sampling values"); resize(); } @@ -201,14 +203,14 @@ TypedImageChannel::slice () const const IMATH_NAMESPACE::Box2i &dw = image().dataWindow(); 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), - sizeof (T), - (w / _xSampling) * sizeof (T), - _xSampling, - _ySampling); + return IMF::Slice::Make ( + pixelType(), + &_pixels[0][0], + dw, + sizeof(T), + (w / _xSampling) * sizeof (T), + _xSampling, + _ySampling); } @@ -227,7 +229,9 @@ template void TypedImageChannel::black () { - memset(&_pixels[0][0],0,image().width()/_xSampling*image().height()/_ySampling*sizeof(T)); + size_t nx = static_cast( image().width() ) / static_cast( _xSampling ); + size_t ny = static_cast( image().height() ) / static_cast( _ySampling ); + memset(&_pixels[0][0],0,nx*ny*sizeof(T)); }