From dfa52f4f7670bee27ad496b6e7d978f17b79da01 Mon Sep 17 00:00:00 2001 From: Peter Hillman Date: Fri, 21 Aug 2020 15:50:23 +1200 Subject: [PATCH 1/4] reduce size limit for scanline files; protect against large chunkoffset allocations Signed-off-by: Peter Hillman --- OpenEXR/IlmImf/ImfCompressor.cpp | 31 ++++++++++++++++++++++ OpenEXR/IlmImf/ImfCompressor.h | 8 ++++++ OpenEXR/IlmImf/ImfMisc.cpp | 33 +----------------------- OpenEXR/IlmImf/ImfMisc.h | 7 +++++ OpenEXR/IlmImf/ImfMultiPartInputFile.cpp | 22 +++++++++++++++- OpenEXR/IlmImf/ImfScanLineInputFile.cpp | 27 ++++++++++++++++--- 6 files changed, 91 insertions(+), 37 deletions(-) diff --git a/OpenEXR/IlmImf/ImfCompressor.cpp b/OpenEXR/IlmImf/ImfCompressor.cpp index 1905a4d66b..1c336a8557 100644 --- a/OpenEXR/IlmImf/ImfCompressor.cpp +++ b/OpenEXR/IlmImf/ImfCompressor.cpp @@ -176,6 +176,37 @@ newCompressor (Compression c, size_t maxScanLineSize, const Header &hdr) } +// for a given compression type, return the number of scanlines +// compressed into a single chunk +// TODO add to API and move to ImfCompressor.cpp +int +numLinesInBuffer(Compression comp) +{ + switch(comp) + { + case NO_COMPRESSION : + case RLE_COMPRESSION: + case ZIPS_COMPRESSION: + return 1; + case ZIP_COMPRESSION: + return 16; + case PIZ_COMPRESSION: + return 32; + case PXR24_COMPRESSION: + return 16; + case B44_COMPRESSION: + case B44A_COMPRESSION: + case DWAA_COMPRESSION: + return 32; + case DWAB_COMPRESSION: + return 256; + + default: + throw IEX_NAMESPACE::ArgExc ("Unknown compression type"); + } +} + + Compressor * newTileCompressor (Compression c, size_t tileLineSize, diff --git a/OpenEXR/IlmImf/ImfCompressor.h b/OpenEXR/IlmImf/ImfCompressor.h index 958677865a..a6850b5f8b 100644 --- a/OpenEXR/IlmImf/ImfCompressor.h +++ b/OpenEXR/IlmImf/ImfCompressor.h @@ -268,6 +268,14 @@ Compressor * newTileCompressor (Compression c, const Header &hdr); +//----------------------------------------------------------------- +// Return the maximum number of scanlines in each chunk +// of a scanline image using the given compression scheme +//----------------------------------------------------------------- + +IMF_EXPORT +int numLinesInBuffer(Compression comp); + OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT #endif diff --git a/OpenEXR/IlmImf/ImfMisc.cpp b/OpenEXR/IlmImf/ImfMisc.cpp index d2c8478770..b397b9f983 100644 --- a/OpenEXR/IlmImf/ImfMisc.cpp +++ b/OpenEXR/IlmImf/ImfMisc.cpp @@ -1848,38 +1848,7 @@ usesLongNames (const Header &header) return false; } -namespace -{ -// for a given compression type, return the number of scanlines -// compressed into a single chunk -// TODO add to API and move to ImfCompressor.cpp -int -numLinesInBuffer(Compression comp) -{ - switch(comp) - { - case NO_COMPRESSION : - case RLE_COMPRESSION: - case ZIPS_COMPRESSION: - return 1; - case ZIP_COMPRESSION: - return 16; - case PIZ_COMPRESSION: - return 32; - case PXR24_COMPRESSION: - return 16; - case B44_COMPRESSION: - case B44A_COMPRESSION: - case DWAA_COMPRESSION: - return 32; - case DWAB_COMPRESSION: - return 256; - - default: - throw IEX_NAMESPACE::ArgExc ("Unknown compression type"); - } -} -} + int getScanlineChunkOffsetTableSize(const Header& header) diff --git a/OpenEXR/IlmImf/ImfMisc.h b/OpenEXR/IlmImf/ImfMisc.h index f1cf648abc..498d18677a 100644 --- a/OpenEXR/IlmImf/ImfMisc.h +++ b/OpenEXR/IlmImf/ImfMisc.h @@ -475,6 +475,13 @@ bool usesLongNames (const Header &header); IMF_EXPORT int getChunkOffsetTableSize(const Header& header,bool deprecated_attribute=false); + +// +// return the number of scanlines in each chunk of a scanlineimage for the given scheme +// +IMF_EXPORT int +numLinesInBuffer(Compression comp); + OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT diff --git a/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp b/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp index 1e04726984..2562828193 100644 --- a/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp +++ b/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp @@ -741,7 +741,9 @@ MultiPartInputFile::Data::getPart(int partNumber) return parts[partNumber]; } - +namespace{ +static const int gLargeChunkTableSize = 1024*1024; +} void MultiPartInputFile::Data::readChunkOffsetTables(bool reconstructChunkOffsetTable) @@ -751,8 +753,26 @@ MultiPartInputFile::Data::readChunkOffsetTables(bool reconstructChunkOffsetTable for (size_t i = 0; i < parts.size(); i++) { int chunkOffsetTableSize = getChunkOffsetTableSize(parts[i]->header); + + // + // avoid allocating excessive memory. + // If the chunktablesize claims to be large, + // check the file is big enough to contain the file before allocating memory + // + if(chunkOffsetTableSize > gLargeChunkTableSize) + { + Int64 pos = is->tellg(); + is->seekg(pos + (gLargeChunkTableSize-1)*sizeof(Int64)); + Int64 temp; + OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*is, temp); + is->seekg(pos); + + } + parts[i]->chunkOffsets.resize(chunkOffsetTableSize); + + for (int j = 0; j < chunkOffsetTableSize; j++) OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*is, parts[i]->chunkOffsets[j]); diff --git a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp index 1ef4eba3f4..ea05c4df8a 100644 --- a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp +++ b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp @@ -1100,7 +1100,7 @@ newLineBufferTask (TaskGroup *group, } - +static const int gLargeChunkTableSize = 1024*1024; } // namespace @@ -1121,7 +1121,9 @@ void ScanLineInputFile::initialize(const Header& header) size_t maxBytesPerLine = bytesPerLineTable (_data->header, _data->bytesPerLine); - if(maxBytesPerLine > INT_MAX) + Compression comp = _data->header.compression(); + + if(maxBytesPerLine*numLinesInBuffer(comp) > INT_MAX) { throw IEX_NAMESPACE::InputExc("maximum bytes per scanline exceeds maximum permissible size"); } @@ -1129,8 +1131,7 @@ void ScanLineInputFile::initialize(const Header& header) for (size_t i = 0; i < _data->lineBuffers.size(); i++) { - _data->lineBuffers[i] = new LineBuffer (newCompressor - (_data->header.compression(), + _data->lineBuffers[i] = new LineBuffer (newCompressor(comp, maxBytesPerLine, _data->header)); } @@ -1156,6 +1157,24 @@ void ScanLineInputFile::initialize(const Header& header) int lineOffsetSize = (dataWindow.max.y - dataWindow.min.y + _data->linesInBuffer) / _data->linesInBuffer; + + + + // + // avoid allocating excessive memory due to large lineOffsets table size + // if the chunktablesize claims to be large, + // check the file is big enough to contain the file before allocating memory + // + if(chunkOffsetTableSize > gLargeChunkTableSize) + { + Int64 pos = is->tellg(); + is->seekg(pos + (gLargeChunkTableSize-1)*sizeof(Int64)); + Int64 temp; + OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*is, temp); + is->seekg(pos); + + } + _data->lineOffsets.resize (lineOffsetSize); } From c72485f7bb5cc8cc5493d61df13ebcd608d1d5f2 Mon Sep 17 00:00:00 2001 From: Peter Hillman Date: Fri, 21 Aug 2020 17:47:29 +1200 Subject: [PATCH 2/4] bugfix for memory limit changes Signed-off-by: Peter Hillman --- OpenEXR/IlmImf/ImfMultiPartInputFile.cpp | 2 +- OpenEXR/IlmImf/ImfScanLineInputFile.cpp | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp b/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp index 2562828193..7213073c9d 100644 --- a/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp +++ b/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp @@ -762,7 +762,7 @@ MultiPartInputFile::Data::readChunkOffsetTables(bool reconstructChunkOffsetTable if(chunkOffsetTableSize > gLargeChunkTableSize) { Int64 pos = is->tellg(); - is->seekg(pos + (gLargeChunkTableSize-1)*sizeof(Int64)); + is->seekg(pos + (chunkOffsetTableSize-1)*sizeof(Int64)); Int64 temp; OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*is, temp); is->seekg(pos); diff --git a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp index ea05c4df8a..4120a9ede0 100644 --- a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp +++ b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp @@ -1161,17 +1161,17 @@ void ScanLineInputFile::initialize(const Header& header) // - // avoid allocating excessive memory due to large lineOffsets table size - // if the chunktablesize claims to be large, + // avoid allocating excessive memory due to large lineOffsets table size. + // If the chunktablesize claims to be large, // check the file is big enough to contain the file before allocating memory // - if(chunkOffsetTableSize > gLargeChunkTableSize) + if(lineOffsetSize > gLargeChunkTableSize) { - Int64 pos = is->tellg(); - is->seekg(pos + (gLargeChunkTableSize-1)*sizeof(Int64)); + Int64 pos = _streamData->is->tellg(); + _streamData->is->seekg(pos + (lineOffsetSize-1)*sizeof(Int64)); Int64 temp; - OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*is, temp); - is->seekg(pos); + OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*_streamData->is, temp); + _streamData->is->seekg(pos); } From 81a54c31c5e8fec6f6ce470f08dece35eec63c5b Mon Sep 17 00:00:00 2001 From: Peter Hillman Date: Mon, 24 Aug 2020 10:33:02 +1200 Subject: [PATCH 3/4] rearrange chunkoffset test to protect bytesperline table too Signed-off-by: Peter Hillman --- OpenEXR/IlmImf/ImfScanLineInputFile.cpp | 55 ++++++++++++++----------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp index 4120a9ede0..1d0077bef5 100644 --- a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp +++ b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp @@ -1118,17 +1118,43 @@ void ScanLineInputFile::initialize(const Header& header) _data->minY = dataWindow.min.y; _data->maxY = dataWindow.max.y; + Compression comp = _data->header.compression(); + + _data->linesInBuffer = + numLinesInBuffer (comp); + + int lineOffsetSize = (dataWindow.max.y - dataWindow.min.y + + _data->linesInBuffer) / _data->linesInBuffer; + + // + // avoid allocating excessive memory due to large lineOffsets table size. + // If the chunktablesize claims to be large, + // check the file is big enough to contain the file before allocating memory + // in the bytesPerLineTable and the lineOffsets table + // + if (lineOffsetSize > gLargeChunkTableSize) + { + Int64 pos = _streamData->is->tellg(); + _streamData->is->seekg(pos + (lineOffsetSize-1)*sizeof(Int64)); + Int64 temp; + OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*_streamData->is, temp); + _streamData->is->seekg(pos); + + } + + size_t maxBytesPerLine = bytesPerLineTable (_data->header, _data->bytesPerLine); - - Compression comp = _data->header.compression(); - if(maxBytesPerLine*numLinesInBuffer(comp) > INT_MAX) + if (maxBytesPerLine*numLinesInBuffer(comp) > INT_MAX) { throw IEX_NAMESPACE::InputExc("maximum bytes per scanline exceeds maximum permissible size"); } + // + // allocate compressor objects + // for (size_t i = 0; i < _data->lineBuffers.size(); i++) { _data->lineBuffers[i] = new LineBuffer (newCompressor(comp, @@ -1136,8 +1162,7 @@ void ScanLineInputFile::initialize(const Header& header) _data->header)); } - _data->linesInBuffer = - numLinesInBuffer (_data->lineBuffers[0]->compressor); + _data->lineBufferSize = maxBytesPerLine * _data->linesInBuffer; @@ -1154,26 +1179,6 @@ void ScanLineInputFile::initialize(const Header& header) _data->linesInBuffer, _data->offsetInLineBuffer); - int lineOffsetSize = (dataWindow.max.y - dataWindow.min.y + - _data->linesInBuffer) / _data->linesInBuffer; - - - - - // - // avoid allocating excessive memory due to large lineOffsets table size. - // If the chunktablesize claims to be large, - // check the file is big enough to contain the file before allocating memory - // - if(lineOffsetSize > gLargeChunkTableSize) - { - Int64 pos = _streamData->is->tellg(); - _streamData->is->seekg(pos + (lineOffsetSize-1)*sizeof(Int64)); - Int64 temp; - OPENEXR_IMF_INTERNAL_NAMESPACE::Xdr::read (*_streamData->is, temp); - _streamData->is->seekg(pos); - - } _data->lineOffsets.resize (lineOffsetSize); } From 03363f84396c2a9994fb2de9227e61fddf4b24a5 Mon Sep 17 00:00:00 2001 From: Peter Hillman Date: Fri, 28 Aug 2020 14:47:44 +1200 Subject: [PATCH 4/4] remove extraneous function declaration; tidy comments Signed-off-by: Peter Hillman --- OpenEXR/IlmImf/ImfMisc.h | 6 ------ OpenEXR/IlmImf/ImfMultiPartInputFile.cpp | 6 ++++-- OpenEXR/IlmImf/ImfScanLineInputFile.cpp | 6 ++++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/OpenEXR/IlmImf/ImfMisc.h b/OpenEXR/IlmImf/ImfMisc.h index 498d18677a..3535ea676b 100644 --- a/OpenEXR/IlmImf/ImfMisc.h +++ b/OpenEXR/IlmImf/ImfMisc.h @@ -476,12 +476,6 @@ IMF_EXPORT int getChunkOffsetTableSize(const Header& header,bool deprecated_attribute=false); -// -// return the number of scanlines in each chunk of a scanlineimage for the given scheme -// -IMF_EXPORT int -numLinesInBuffer(Compression comp); - OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT diff --git a/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp b/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp index 7213073c9d..689956c90a 100644 --- a/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp +++ b/OpenEXR/IlmImf/ImfMultiPartInputFile.cpp @@ -757,9 +757,11 @@ MultiPartInputFile::Data::readChunkOffsetTables(bool reconstructChunkOffsetTable // // avoid allocating excessive memory. // If the chunktablesize claims to be large, - // check the file is big enough to contain the file before allocating memory + // check the file is big enough to contain the table before allocating memory. + // Attempt to read the last entry in the table. Either the seekg() or the read() + // call will throw an exception if the file is too small to contain the table // - if(chunkOffsetTableSize > gLargeChunkTableSize) + if (chunkOffsetTableSize > gLargeChunkTableSize) { Int64 pos = is->tellg(); is->seekg(pos + (chunkOffsetTableSize-1)*sizeof(Int64)); diff --git a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp index 1d0077bef5..b020cb9bb5 100644 --- a/OpenEXR/IlmImf/ImfScanLineInputFile.cpp +++ b/OpenEXR/IlmImf/ImfScanLineInputFile.cpp @@ -1129,8 +1129,10 @@ void ScanLineInputFile::initialize(const Header& header) // // avoid allocating excessive memory due to large lineOffsets table size. // If the chunktablesize claims to be large, - // check the file is big enough to contain the file before allocating memory - // in the bytesPerLineTable and the lineOffsets table + // check the file is big enough to contain the table before allocating memory + // in the bytesPerLineTable and the lineOffsets table. + // Attempt to read the last entry in the table. Either the seekg() or the read() + // call will throw an exception if the file is too small to contain the table // if (lineOffsetSize > gLargeChunkTableSize) {