From 7a20c2cdfcf3e45f31063b4eaff94f1c0776b593 Mon Sep 17 00:00:00 2001 From: Florian Kainz Date: Wed, 29 Jul 2009 02:18:59 +0000 Subject: [PATCH] Bug fix: crashes when reading damaged images, caused by buffer overruns, or by deleting uninitialized pointers (found by Apple). ImfB44Compressor.cpp | 15 +++- ImfB44Compressor.h | 4 - ImfCheckedArithmetic.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++ ImfCompressor.cpp | 9 +- ImfCompressor.h | 7 +- ImfHuf.cpp | 20 +++++- ImfPizCompressor.cpp | 18 ++++- ImfPizCompressor.h | 5 + ImfPreviewImage.cpp | 4 - ImfPxr24Compressor.cpp | 15 +++- ImfPxr24Compressor.h | 5 + ImfRleCompressor.cpp | 5 - ImfRleCompressor.h | 2 ImfZipCompressor.cpp | 17 +++-- ImfZipCompressor.h | 5 + Makefile.am | 7 +- 16 files changed, 261 insertions(+), 38 deletions(-) Index: openexr-1.6.1/IlmImf/ImfB44Compressor.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfB44Compressor.cpp +++ openexr-1.6.1/IlmImf/ImfB44Compressor.cpp @@ -101,6 +101,7 @@ #include #include #include +#include #include #include #include @@ -465,8 +466,8 @@ struct B44Compressor::ChannelData B44Compressor::B44Compressor (const Header &hdr, - int maxScanLineSize, - int numScanLines, + size_t maxScanLineSize, + size_t numScanLines, bool optFlatFields) : Compressor (hdr), @@ -487,7 +488,9 @@ B44Compressor::B44Compressor // if uncompressed pixel data should be in native or Xdr format. // - _tmpBuffer = new unsigned short [maxScanLineSize * numScanLines]; + _tmpBuffer = new unsigned short + [checkArraySize (uiMult (maxScanLineSize, numScanLines), + sizeof (unsigned short))]; const ChannelList &channels = header().channels(); int numHalfChans = 0; @@ -507,9 +510,11 @@ B44Compressor::B44Compressor // Compressed data may be larger than the input data // - int padding = 12 * numHalfChans * (numScanLines + 3) / 4; + size_t padding = 12 * numHalfChans * (numScanLines + 3) / 4; + + _outBuffer = new char + [uiAdd (uiMult (maxScanLineSize, numScanLines), padding)]; - _outBuffer = new char [maxScanLineSize * numScanLines + padding]; _channelData = new ChannelData[_numChans]; int i = 0; Index: openexr-1.6.1/IlmImf/ImfB44Compressor.h =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfB44Compressor.h +++ openexr-1.6.1/IlmImf/ImfB44Compressor.h @@ -54,8 +54,8 @@ class B44Compressor: public Compressor public: B44Compressor (const Header &hdr, - int maxScanLineSize, - int numScanLines, + size_t maxScanLineSize, + size_t numScanLines, bool optFlatFields); virtual ~B44Compressor (); Index: openexr-1.6.1/IlmImf/ImfCheckedArithmetic.h =================================================================== --- /dev/null +++ openexr-1.6.1/IlmImf/ImfCheckedArithmetic.h @@ -0,0 +1,161 @@ +/////////////////////////////////////////////////////////////////////////// +// +// Copyright (c) 2009, Industrial Light & Magic, a division of Lucas +// Digital Ltd. LLC +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Industrial Light & Magic nor the names of +// its contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +/////////////////////////////////////////////////////////////////////////// + +#ifndef INCLUDED_IMF_CHECKED_ARITHMETIC_H +#define INCLUDED_IMF_CHECKED_ARITHMETIC_H + +//----------------------------------------------------------------------------- +// +// Integer arithmetic operations that throw exceptions +// on overflow, underflow or division by zero. +// +//----------------------------------------------------------------------------- + +#include +#include + +namespace Imf { + +template struct StaticAssertionFailed; +template <> struct StaticAssertionFailed {}; + +#define IMF_STATIC_ASSERT(x) \ + do {StaticAssertionFailed staticAssertionFailed;} while (false) + + +template +T +uiMult (T a, T b) +{ + // + // Unsigned integer multiplication + // + + IMF_STATIC_ASSERT (!std::numeric_limits::is_signed && + std::numeric_limits::is_integer); + + if (a > 0 && b > std::numeric_limits::max() / a) + throw Iex::OverflowExc ("Integer multiplication overflow."); + + return a * b; +} + + +template +T +uiDiv (T a, T b) +{ + // + // Unsigned integer division + // + + IMF_STATIC_ASSERT (!std::numeric_limits::is_signed && + std::numeric_limits::is_integer); + + if (b == 0) + throw Iex::DivzeroExc ("Integer division by zero."); + + return a / b; +} + + +template +T +uiAdd (T a, T b) +{ + // + // Unsigned integer addition + // + + IMF_STATIC_ASSERT (!std::numeric_limits::is_signed && + std::numeric_limits::is_integer); + + if (a > std::numeric_limits::max() - b) + throw Iex::OverflowExc ("Integer addition overflow."); + + return a + b; +} + + +template +T +uiSub (T a, T b) +{ + // + // Unsigned integer subtraction + // + + IMF_STATIC_ASSERT (!std::numeric_limits::is_signed && + std::numeric_limits::is_integer); + + if (a < b) + throw Iex::UnderflowExc ("Integer subtraction underflow."); + + return a - b; +} + + +template +size_t +checkArraySize (T n, size_t s) +{ + // + // Verify that the size, in bytes, of an array with n elements + // of size s can be computed without overflowing: + // + // If computing + // + // size_t (n) * s + // + // would overflow, then throw an Iex::OverflowExc exception. + // Otherwise return + // + // size_t (n). + // + + IMF_STATIC_ASSERT (!std::numeric_limits::is_signed && + std::numeric_limits::is_integer); + + IMF_STATIC_ASSERT (sizeof (T) <= sizeof (size_t)); + + if (size_t (n) > std::numeric_limits::max() / s) + throw Iex::OverflowExc ("Integer multiplication overflow."); + + return size_t (n); +} + + +} // namespace Imf + +#endif Index: openexr-1.6.1/IlmImf/ImfCompressor.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfCompressor.cpp +++ openexr-1.6.1/IlmImf/ImfCompressor.cpp @@ -46,6 +46,7 @@ #include #include #include +#include namespace Imf { @@ -109,7 +110,7 @@ isValidCompression (Compression c) Compressor * -newCompressor (Compression c, int maxScanLineSize, const Header &hdr) +newCompressor (Compression c, size_t maxScanLineSize, const Header &hdr) { switch (c) { @@ -150,15 +151,15 @@ newCompressor (Compression c, int maxSca Compressor * newTileCompressor (Compression c, - int tileLineSize, - int numTileLines, + size_t tileLineSize, + size_t numTileLines, const Header &hdr) { switch (c) { case RLE_COMPRESSION: - return new RleCompressor (hdr, tileLineSize * numTileLines); + return new RleCompressor (hdr, uiMult (tileLineSize, numTileLines)); case ZIPS_COMPRESSION: case ZIP_COMPRESSION: Index: openexr-1.6.1/IlmImf/ImfCompressor.h =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfCompressor.h +++ openexr-1.6.1/IlmImf/ImfCompressor.h @@ -45,6 +45,7 @@ #include #include "ImathBox.h" +#include namespace Imf { @@ -219,7 +220,7 @@ bool isValidCompression (Compression c) //----------------------------------------------------------------- Compressor * newCompressor (Compression c, - int maxScanLineSize, + size_t maxScanLineSize, const Header &hdr); @@ -241,8 +242,8 @@ Compressor * newCompressor (Compression //----------------------------------------------------------------- Compressor * newTileCompressor (Compression c, - int tileLineSize, - int numTileLines, + size_t tileLineSize, + size_t numTileLines, const Header &hdr); Index: openexr-1.6.1/IlmImf/ImfHuf.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfHuf.cpp +++ openexr-1.6.1/IlmImf/ImfHuf.cpp @@ -579,6 +579,19 @@ hufUnpackEncTable // // +// Clear a newly allocated decoding table so that it contains only zeroes. +// + +void +hufClearDecTable + (HufDec * hdecod) // io: (allocated by caller) + // decoding table [HUF_DECSIZE] +{ + memset (hdecod, 0, sizeof (HufDec) * HUF_DECSIZE); +} + + +// // Build a decoding hash table based on the encoding table hcode: // - short codes (<= HUF_DECBITS) are resolved with a single table access; // - long code entry allocations are not optimized, because long codes are @@ -595,11 +608,10 @@ hufBuildDecTable // decoding table [HUF_DECSIZE] { // - // Init hashtable & loop on all codes + // Init hashtable & loop on all codes. + // Assumes that hufClearDecTable(hdecod) has already been called. // - memset (hdecod, 0, sizeof (HufDec) * HUF_DECSIZE); - for (; im <= iM; im++) { Int64 c = hufCode (hcode[im]); @@ -1049,6 +1061,8 @@ hufUncompress (const char compressed[], AutoArray freq; AutoArray hdec; + hufClearDecTable (hdec); + hufUnpackEncTable (&ptr, nCompressed - (ptr - compressed), im, iM, freq); try Index: openexr-1.6.1/IlmImf/ImfPizCompressor.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfPizCompressor.cpp +++ openexr-1.6.1/IlmImf/ImfPizCompressor.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -168,8 +169,8 @@ struct PizCompressor::ChannelData PizCompressor::PizCompressor (const Header &hdr, - int maxScanLineSize, - int numScanLines) + size_t maxScanLineSize, + size_t numScanLines) : Compressor (hdr), _maxScanLineSize (maxScanLineSize), @@ -181,8 +182,17 @@ PizCompressor::PizCompressor _channels (hdr.channels()), _channelData (0) { - _tmpBuffer = new unsigned short [maxScanLineSize * numScanLines / 2]; - _outBuffer = new char [maxScanLineSize * numScanLines + 65536 + 8192]; + size_t tmpBufferSize = + uiMult (maxScanLineSize, numScanLines) / 2; + + size_t outBufferSize = + uiAdd (uiMult (maxScanLineSize, numScanLines), + size_t (65536 + 8192)); + + _tmpBuffer = new unsigned short + [checkArraySize (tmpBufferSize, sizeof (unsigned short))]; + + _outBuffer = new char [outBufferSize]; const ChannelList &channels = header().channels(); bool onlyHalfChannels = true; Index: openexr-1.6.1/IlmImf/ImfPizCompressor.h =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfPizCompressor.h +++ openexr-1.6.1/IlmImf/ImfPizCompressor.h @@ -53,7 +53,10 @@ class PizCompressor: public Compressor { public: - PizCompressor (const Header &hdr, int maxScanLineSize, int numScanLines); + PizCompressor (const Header &hdr, + size_t maxScanLineSize, + size_t numScanLines); + virtual ~PizCompressor (); virtual int numScanLines () const; Index: openexr-1.6.1/IlmImf/ImfPreviewImage.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfPreviewImage.cpp +++ openexr-1.6.1/IlmImf/ImfPreviewImage.cpp @@ -40,6 +40,7 @@ //----------------------------------------------------------------------------- #include +#include #include "Iex.h" namespace Imf { @@ -51,7 +52,8 @@ PreviewImage::PreviewImage (unsigned int { _width = width; _height = height; - _pixels = new PreviewRgba [_width * _height]; + _pixels = new PreviewRgba + [checkArraySize (uiMult (_width, _height), sizeof (PreviewRgba))]; if (pixels) { Index: openexr-1.6.1/IlmImf/ImfPxr24Compressor.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfPxr24Compressor.cpp +++ openexr-1.6.1/IlmImf/ImfPxr24Compressor.cpp @@ -67,6 +67,7 @@ #include #include #include +#include #include #include #include @@ -175,8 +176,8 @@ tooMuchData () Pxr24Compressor::Pxr24Compressor (const Header &hdr, - int maxScanLineSize, - int numScanLines) + size_t maxScanLineSize, + size_t numScanLines) : Compressor (hdr), _maxScanLineSize (maxScanLineSize), @@ -185,10 +186,16 @@ Pxr24Compressor::Pxr24Compressor (const _outBuffer (0), _channels (hdr.channels()) { - int maxInBytes = maxScanLineSize * numScanLines; + size_t maxInBytes = + uiMult (maxScanLineSize, numScanLines); + + size_t maxOutBytes = + uiAdd (uiAdd (maxInBytes, + size_t (ceil (maxInBytes * 0.01))), + size_t (100)); _tmpBuffer = new unsigned char [maxInBytes]; - _outBuffer = new char [int (ceil (maxInBytes * 1.01)) + 100]; + _outBuffer = new char [maxOutBytes]; const Box2i &dataWindow = hdr.dataWindow(); Index: openexr-1.6.1/IlmImf/ImfPxr24Compressor.h =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfPxr24Compressor.h +++ openexr-1.6.1/IlmImf/ImfPxr24Compressor.h @@ -51,7 +51,10 @@ class Pxr24Compressor: public Compressor { public: - Pxr24Compressor (const Header &hdr, int maxScanLineSize, int numScanLines); + Pxr24Compressor (const Header &hdr, + size_t maxScanLineSize, + size_t numScanLines); + virtual ~Pxr24Compressor (); virtual int numScanLines () const; Index: openexr-1.6.1/IlmImf/ImfRleCompressor.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfRleCompressor.cpp +++ openexr-1.6.1/IlmImf/ImfRleCompressor.cpp @@ -41,6 +41,7 @@ //----------------------------------------------------------------------------- #include +#include #include "Iex.h" namespace Imf { @@ -158,14 +159,14 @@ rleUncompress (int inLength, int maxLeng } // namespace -RleCompressor::RleCompressor (const Header &hdr, int maxScanLineSize): +RleCompressor::RleCompressor (const Header &hdr, size_t maxScanLineSize): Compressor (hdr), _maxScanLineSize (maxScanLineSize), _tmpBuffer (0), _outBuffer (0) { _tmpBuffer = new char [maxScanLineSize]; - _outBuffer = new char [maxScanLineSize * 3 / 2]; + _outBuffer = new char [uiMult (maxScanLineSize, size_t (3)) / 2]; } Index: openexr-1.6.1/IlmImf/ImfRleCompressor.h =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfRleCompressor.h +++ openexr-1.6.1/IlmImf/ImfRleCompressor.h @@ -52,7 +52,7 @@ class RleCompressor: public Compressor { public: - RleCompressor (const Header &hdr, int maxScanLineSize); + RleCompressor (const Header &hdr, size_t maxScanLineSize); virtual ~RleCompressor (); virtual int numScanLines () const; Index: openexr-1.6.1/IlmImf/ImfZipCompressor.cpp =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfZipCompressor.cpp +++ openexr-1.6.1/IlmImf/ImfZipCompressor.cpp @@ -41,6 +41,7 @@ //----------------------------------------------------------------------------- #include +#include #include "Iex.h" #include @@ -49,8 +50,8 @@ namespace Imf { ZipCompressor::ZipCompressor (const Header &hdr, - int maxScanLineSize, - int numScanLines) + size_t maxScanLineSize, + size_t numScanLines) : Compressor (hdr), _maxScanLineSize (maxScanLineSize), @@ -58,11 +59,19 @@ ZipCompressor::ZipCompressor _tmpBuffer (0), _outBuffer (0) { + size_t maxInBytes = + uiMult (maxScanLineSize, numScanLines); + + size_t maxOutBytes = + uiAdd (uiAdd (maxInBytes, + size_t (ceil (maxInBytes * 0.01))), + size_t (100)); + _tmpBuffer = - new char [maxScanLineSize * numScanLines]; + new char [maxInBytes]; _outBuffer = - new char [int (ceil (maxScanLineSize * numScanLines * 1.01)) + 100]; + new char [maxOutBytes]; } Index: openexr-1.6.1/IlmImf/ImfZipCompressor.h =================================================================== --- openexr-1.6.1.orig/IlmImf/ImfZipCompressor.h +++ openexr-1.6.1/IlmImf/ImfZipCompressor.h @@ -52,7 +52,10 @@ class ZipCompressor: public Compressor { public: - ZipCompressor (const Header &hdr, int maxScanLineSize, int numScanLines); + ZipCompressor (const Header &hdr, + size_t maxScanLineSize, + size_t numScanLines); + virtual ~ZipCompressor (); virtual int numScanLines () const; Index: openexr-1.6.1/IlmImf/Makefile.am =================================================================== --- openexr-1.6.1.orig/IlmImf/Makefile.am +++ openexr-1.6.1/IlmImf/Makefile.am @@ -56,7 +56,9 @@ libIlmImf_la_SOURCES = ImfAttribute.cpp ImfTiledOutputFile.h ImfTiledRgbaFile.h \ ImfRgbaYca.cpp ImfRgbaYca.h \ ImfPxr24Compressor.cpp ImfPxr24Compressor.h \ - ImfTestFile.cpp ImfTestFile.h ImfThreading.h + ImfTestFile.cpp ImfTestFile.h ImfThreading.h \ + ImfCheckedArithmetic.h + libIlmImf_la_LDFLAGS = @ILMBASE_LDFLAGS@ -version-info @LIBTOOL_VERSION@ \ -no-undefined @@ -99,7 +101,8 @@ libIlmImfinclude_HEADERS = ImfAttribute. noinst_HEADERS = ImfCompressor.h ImfRleCompressor.h ImfZipCompressor.h \ ImfPizCompressor.h ImfMisc.h ImfAutoArray.h ImfTiledMisc.h \ - ImfTileOffsets.h ImfScanLineInputFile.h ImfPxr24Compressor.h + ImfTileOffsets.h ImfScanLineInputFile.h ImfPxr24Compressor.h \ + ImfCheckedArithmetic.h EXTRA_DIST = $(noinst_HEADERS) b44ExpLogTable.cpp b44ExpLogTable.h