Skip to content

Commit

Permalink
fix memory leaks and invalid memory accesses
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
  • Loading branch information
@peterhillman @cary-ilm
peterhillman authored and cary-ilm committed Feb 10, 2020
1 parent 031199c commit e79d229
Show file tree
Hide file tree
Showing 18 changed files with 426 additions and 166 deletions.
3 changes: 2 additions & 1 deletion OpenEXR/IlmImf/ImfCompositeDeepScanLine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@

#include <Iex.h>
#include <vector>
#include <stddef.h>
OPENEXR_IMF_INTERNAL_NAMESPACE_SOURCE_ENTER

using std::vector;
Expand Down Expand Up @@ -179,7 +180,7 @@ CompositeDeepScanLine::Data::handleDeepFrameBuffer (DeepFrameBuffer& buf,
int start,
int end)
{
int width=_dataWindow.size().x+1;
ptrdiff_t width=_dataWindow.size().x+1;
size_t pixelcount = width * (end-start+1);
pointers.resize(_channels.size());
counts.resize(pixelcount);
Expand Down
60 changes: 50 additions & 10 deletions OpenEXR/IlmImf/ImfDeepScanLineInputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -914,8 +914,7 @@ void DeepScanLineInputFile::initialize(const Header& header)
}
catch (...)
{
delete _data;
_data=NULL;
// Don't delete _data here, leave that to caller
throw;
}
}
Expand All @@ -931,8 +930,15 @@ DeepScanLineInputFile::DeepScanLineInputFile(InputPartData* part)
_data->memoryMapped = _data->_streamData->is->isMemoryMapped();
_data->version = part->version;

initialize(part->header);

try
{
initialize(part->header);
}
catch(...)
{
delete _data;
throw;
}
_data->lineOffsets = part->chunkOffsets;

_data->partNumber = part->partNumber;
Expand All @@ -944,7 +950,6 @@ DeepScanLineInputFile::DeepScanLineInputFile
:
_data (new Data (numThreads))
{
_data->_streamData = new InputStreamMutex();
_data->_deleteStream = true;
OPENEXR_IMF_INTERNAL_NAMESPACE::IStream* is = 0;

Expand All @@ -954,12 +959,29 @@ DeepScanLineInputFile::DeepScanLineInputFile
readMagicNumberAndVersionField(*is, _data->version);
//
// Backward compatibility to read multpart file.
//
// multiPartInitialize will create _streamData
if (isMultiPart(_data->version))
{
compatibilityInitialize(*is);
return;
}
}
catch (IEX_NAMESPACE::BaseExc &e)
{
if (is) delete is;
if (_data) delete _data;

REPLACE_EXC (e, "Cannot read image file "
"\"" << fileName << "\". " << e.what());
throw;
}

//
// not multiPart - allocate stream data and intialise as normal
//
try
{
_data->_streamData = new InputStreamMutex();
_data->_streamData->is = is;
_data->memoryMapped = is->isMemoryMapped();
_data->header.readFrom (*_data->_streamData->is, _data->version);
Expand All @@ -975,7 +997,10 @@ DeepScanLineInputFile::DeepScanLineInputFile
catch (IEX_NAMESPACE::BaseExc &e)
{
if (is) delete is;
if (_data && _data->_streamData) delete _data->_streamData;
if (_data && _data->_streamData)
{
delete _data->_streamData;
}
if (_data) delete _data;

REPLACE_EXC (e, "Cannot read image file "
Expand All @@ -985,7 +1010,10 @@ DeepScanLineInputFile::DeepScanLineInputFile
catch (...)
{
if (is) delete is;
if (_data && _data->_streamData) delete _data->_streamData;
if (_data && _data->_streamData)
{
delete _data->_streamData;
}
if (_data) delete _data;

throw;
Expand All @@ -1009,7 +1037,18 @@ DeepScanLineInputFile::DeepScanLineInputFile

_data->version =version;

initialize (header);
try
{
initialize (header);
}
catch (...)
{
if (_data && _data->_streamData)
{
delete _data->_streamData;
}
if (_data) delete _data;
}

readLineOffsets (*_data->_streamData->is,
_data->lineOrder,
Expand Down Expand Up @@ -1041,8 +1080,9 @@ DeepScanLineInputFile::~DeepScanLineInputFile ()
//

if (_data->partNumber == -1 && _data->_streamData)
{
delete _data->_streamData;

}
delete _data;
}
}
Expand Down
15 changes: 13 additions & 2 deletions OpenEXR/IlmImf/ImfDeepTiledInputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ DeepTiledInputFile::Data::Data (int numThreads):
multiPartBackwardSupport(false),
numThreads(numThreads),
memoryMapped(false),
_streamData(NULL),
sampleCountTableComp(nullptr),
_streamData(nullptr),
_deleteStream(false)
{
//
Expand All @@ -308,6 +309,8 @@ DeepTiledInputFile::Data::~Data ()

for (size_t i = 0; i < slices.size(); i++)
delete slices[i];

delete sampleCountTableComp;
}


Expand Down Expand Up @@ -878,7 +881,15 @@ DeepTiledInputFile::DeepTiledInputFile (InputPartData* part) :
_data (new Data (part->numThreads))
{
_data->_deleteStream=false;
multiPartInitialize(part);
try
{
multiPartInitialize(part);
}
catch(...)
{
delete _data;
throw;
}
}


Expand Down
15 changes: 12 additions & 3 deletions OpenEXR/IlmImf/ImfDwaCompressor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,9 @@ struct DwaCompressor::Classifier
" (truncated rule).");

{
char suffix[Name::SIZE];
memset (suffix, 0, Name::SIZE);
// maximum length of string plus one byte for terminating NULL
char suffix[Name::SIZE+1];
memset (suffix, 0, Name::SIZE+1);
Xdr::read<CharPtrIO> (ptr, std::min(size, Name::SIZE-1), suffix);
_suffix = std::string(suffix);
}
Expand Down Expand Up @@ -2419,7 +2420,7 @@ DwaCompressor::uncompress
unsigned short ruleSize = 0;
Xdr::read<CharPtrIO>(dataPtr, ruleSize);

if (ruleSize < 0)
if (ruleSize < Xdr::size<unsigned short>() )
throw IEX_NAMESPACE::InputExc("Error uncompressing DWA data"
" (corrupt header file).");

Expand Down Expand Up @@ -2814,6 +2815,14 @@ DwaCompressor::uncompress
if (IMATH_NAMESPACE::modp (y, cd->ySampling) != 0)
continue;

//
// sanity check for buffer data lying within range
//
if (cd->planarUncBufferEnd + dstScanlineSize - _planarUncBuffer[UNKNOWN] > _planarUncBufferSize[UNKNOWN] )
{
throw Iex::InputExc("DWA data corrupt");
}

memcpy (rowPtrs[chan][row],
cd->planarUncBufferEnd,
dstScanlineSize);
Expand Down
19 changes: 17 additions & 2 deletions OpenEXR/IlmImf/ImfFastHuf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,29 @@ FastHufDecoder::FastHufDecoder
int symbol = *i >> 6;

if (mapping[codeLen] >= static_cast<Int64>(_numSymbols))
{
delete[] _idToSymbol;
_idToSymbol = NULL;
throw IEX_NAMESPACE::InputExc ("Huffman decode error "
"(Invalid symbol in header).");

}
_idToSymbol[mapping[codeLen]] = symbol;
mapping[codeLen]++;
}

buildTables(base, offset);
//
// exceptions can be thrown whilst building tables. Delete
// _idToSynmbol before re-throwing to prevent memory leak
//
try
{
buildTables(base, offset);
}catch(...)
{
delete[] _idToSymbol;
_idToSymbol = NULL;
throw;
}
}


Expand Down
11 changes: 8 additions & 3 deletions OpenEXR/IlmImf/ImfHeader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
}

const std::string & part_type=hasType() ? type() : "";


if(part_type!="" && !isSupportedType(part_type))
{
Expand All @@ -882,6 +883,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
return;
}

bool isDeep = isDeepData(part_type);

//
// If the file is tiled, verify that the tile description has reasonable
Expand All @@ -902,7 +904,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const

const TileDescription &tileDesc = tileDescription();

if (tileDesc.xSize <= 0 || tileDesc.ySize <= 0)
if (tileDesc.xSize <= 0 || tileDesc.ySize <= 0 || tileDesc.xSize > INT_MAX || tileDesc.ySize > INT_MAX )
throw IEX_NAMESPACE::ArgExc ("Invalid tile size in image header.");

if (maxTileWidth > 0 &&
Expand Down Expand Up @@ -953,7 +955,8 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
if (!isValidCompression (this->compression()))
throw IEX_NAMESPACE::ArgExc ("Unknown compression type in image header.");

if(isDeepData(part_type))

if( isDeep )
{
if (!isValidDeepCompression (this->compression()))
throw IEX_NAMESPACE::ArgExc ("Compression type in header not valid for deep data");
Expand All @@ -965,6 +968,8 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
// If the file is tiled then for each channel, the type must be one of the
// predefined values, and the x and y sampling must both be 1.
//
// x and y sampling must currently also be 1 for deep scanline images
//
// If the file is not tiled then for each channel, the type must be one
// of the predefined values, the x and y coordinates of the data window's
// upper left corner must be divisible by the x and y subsampling factors,
Expand All @@ -974,7 +979,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const

const ChannelList &channels = this->channels();

if (isTiled)
if (isTiled || isDeep)
{
for (ChannelList::ConstIterator i = channels.begin();
i != channels.end();
Expand Down
11 changes: 10 additions & 1 deletion OpenEXR/IlmImf/ImfHuf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,10 @@ hufUncompress (const char compressed[],
unsigned short raw[],
int nRaw)
{
if (nCompressed == 0)
//
// need at least 20 bytes for header
//
if (nCompressed < 20 )
{
if (nRaw != 0)
notEnoughData();
Expand All @@ -1070,6 +1073,12 @@ hufUncompress (const char compressed[],

const char *ptr = compressed + 20;

if ( ptr + (nBits+7 )/8 > compressed+nCompressed)
{
notEnoughData();
return;
}

//
// Fast decoder needs at least 2x64-bits of compressed data, and
// needs to be run-able on this platform. Otherwise, fall back
Expand Down
10 changes: 9 additions & 1 deletion OpenEXR/IlmImf/ImfInputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,15 @@ InputFile::InputFile (InputPartData* part) :
_data (new Data (part->numThreads))
{
_data->_deleteStream=false;
multiPartInitialize (part);
try
{
multiPartInitialize (part);
}
catch(...)
{
delete _data;
throw;
}
}


Expand Down
Loading

0 comments on commit e79d229

Please sign in to comment.