Cherry-pick PRs from master branch which fix issues reported by fuzz …

…tests (#875) * ignore unused bits in B44 mode detection Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #832: use unsigned values in shift to prevent undefined behavior Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #818: compute Huf codelengths using 64 bit to prevent shift overflow Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #817: double-check unpackedBuffer created in DWA uncompress Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #820: suppress sanitizer warnings when writing invalid enums Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #825: Avoid overflow in calculateNumTiles when size=MAX_INT Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #826: restrict maximum tile size to INT_MAX byte limit Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #827: lighter weight reading of Luma-only images via RgbaInputFile Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * refactor channel filling in InputFile API with tiled source Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * handle edge-case of empty framebuffer Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #829: fix buffer overflow check in PIZ decompression Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * Use Int64 in dataWindowForTile to prevent integer overflow (#831) * Use Int64 in dataWindowForTile to prevent integer overflow Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * use signed 64 bit instead for dataWindow calculation Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * prevent overflow in hufUncompress if nBits is large (#836) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * add sanity check for reading multipart files with no parts (#840) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * reduce B44 _tmpBufferSize (was allocating two bytes per byte) (#843) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * check for valid Huf code lengths (#849) * check for valid Huf code lengths * test non-fast huf decoder in testHuf Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * check 1 part files with 'nonimage' bit have type attribute (#860) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * Fix overflow computing deeptile sample table size (#861) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * re-order shift/compare in FastHuf to prevent undefined shift overflow warning (#819) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * more elegant exception handling in exrmaketiled (#841) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * check EXRAllocAligned succeeded to allocate ScanlineInputFile lineBuffers (#844) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * test channels are DCT compressed before DWA decompression (#845) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * Merge ABI-compatible changes from #842 Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com>
AcademySoftwareFoundation · Dec 30, 2020 · 0c2b46f · 0c2b46f
1 parent c32f82c
commit 0c2b46f
Show file tree

Hide file tree

Showing 20 changed files with 606 additions and 249 deletions.
diff --git a/OpenEXR/IlmImf/ImfB44Compressor.cpp b/OpenEXR/IlmImf/ImfB44Compressor.cpp
@@ -381,26 +381,26 @@ unpack14 (const unsigned char b[14], unsigned short s[16])
     s[ 0] = (b[0] << 8) | b[1];
 
     unsigned short shift = (b[ 2] >> 2);
-    unsigned short bias = (0x20 << shift);
+    unsigned short bias = (0x20u << shift);
 
-    s[ 4] = s[ 0] + ((((b[ 2] << 4) | (b[ 3] >> 4)) & 0x3f) << shift) - bias;
-    s[ 8] = s[ 4] + ((((b[ 3] << 2) | (b[ 4] >> 6)) & 0x3f) << shift) - bias;
-    s[12] = s[ 8] +   ((b[ 4]                       & 0x3f) << shift) - bias;
+    s[ 4] = s[ 0] + ((((b[ 2] << 4) | (b[ 3] >> 4)) & 0x3fu) << shift) - bias;
+    s[ 8] = s[ 4] + ((((b[ 3] << 2) | (b[ 4] >> 6)) & 0x3fu) << shift) - bias;
+    s[12] = s[ 8] +   ((b[ 4]                       & 0x3fu) << shift) - bias;
 
-    s[ 1] = s[ 0] +   ((b[ 5] >> 2)                         << shift) - bias;
-    s[ 5] = s[ 4] + ((((b[ 5] << 4) | (b[ 6] >> 4)) & 0x3f) << shift) - bias;
-    s[ 9] = s[ 8] + ((((b[ 6] << 2) | (b[ 7] >> 6)) & 0x3f) << shift) - bias;
-    s[13] = s[12] +   ((b[ 7]                       & 0x3f) << shift) - bias;
+    s[ 1] = s[ 0] +   ((unsigned int) (b[ 5] >> 2)           << shift) - bias;
+    s[ 5] = s[ 4] + ((((b[ 5] << 4) | (b[ 6] >> 4)) & 0x3fu) << shift) - bias;
+    s[ 9] = s[ 8] + ((((b[ 6] << 2) | (b[ 7] >> 6)) & 0x3fu) << shift) - bias;
+    s[13] = s[12] +   ((b[ 7]                       & 0x3fu) << shift) - bias;
 
-    s[ 2] = s[ 1] +   ((b[ 8] >> 2)                         << shift) - bias;
-    s[ 6] = s[ 5] + ((((b[ 8] << 4) | (b[ 9] >> 4)) & 0x3f) << shift) - bias;
-    s[10] = s[ 9] + ((((b[ 9] << 2) | (b[10] >> 6)) & 0x3f) << shift) - bias;
-    s[14] = s[13] +   ((b[10]                       & 0x3f) << shift) - bias;
+    s[ 2] = s[ 1] +   ((unsigned int)(b[ 8] >> 2)            << shift) - bias;
+    s[ 6] = s[ 5] + ((((b[ 8] << 4) | (b[ 9] >> 4)) & 0x3fu) << shift) - bias;
+    s[10] = s[ 9] + ((((b[ 9] << 2) | (b[10] >> 6)) & 0x3fu) << shift) - bias;
+    s[14] = s[13] +   ((b[10]                       & 0x3fu) << shift) - bias;
 
-    s[ 3] = s[ 2] +   ((b[11] >> 2)                         << shift) - bias;
-    s[ 7] = s[ 6] + ((((b[11] << 4) | (b[12] >> 4)) & 0x3f) << shift) - bias;
-    s[11] = s[10] + ((((b[12] << 2) | (b[13] >> 6)) & 0x3f) << shift) - bias;
-    s[15] = s[14] +   ((b[13]                       & 0x3f) << shift) - bias;
+    s[ 3] = s[ 2] +   ((unsigned int)(b[11] >> 2)            << shift) - bias;
+    s[ 7] = s[ 6] + ((((b[11] << 4) | (b[12] >> 4)) & 0x3fu) << shift) - bias;
+    s[11] = s[10] + ((((b[12] << 2) | (b[13] >> 6)) & 0x3fu) << shift) - bias;
+    s[15] = s[14] +   ((b[13]                       & 0x3fu) << shift) - bias;
 
     for (int i = 0; i < 16; ++i)
     {
@@ -494,7 +494,7 @@ B44Compressor::B44Compressor
     //
 
     _tmpBuffer = new unsigned short
-        [checkArraySize (uiMult (maxScanLineSize, numScanLines),
+        [checkArraySize (uiMult (maxScanLineSize / sizeof(unsigned short), numScanLines),
                          sizeof (unsigned short))];
 
     const ChannelList &channels = header().channels();
@@ -951,7 +951,10 @@ B44Compressor::uncompress (const char *inPtr,
 		if (inSize < 3)
 		    notEnoughData();
 
-		if (((const unsigned char *)inPtr)[2] == 0xfc)
+                //
+                // If shift exponent is 63, call unpack14 (ignoring unused bits)
+                //
+		if (((const unsigned char *)inPtr)[2] >= (13<<2) )
 		{
 		    unpack3 ((const unsigned char *)inPtr, s);
 		    inPtr += 3;

diff --git a/OpenEXR/IlmImf/ImfDeepImageStateAttribute.cpp b/OpenEXR/IlmImf/ImfDeepImageStateAttribute.cpp
@@ -58,6 +58,12 @@ template <>
 void
 DeepImageStateAttribute::writeValueTo
     (OPENEXR_IMF_INTERNAL_NAMESPACE::OStream &os, int version) const
+#if defined (__clang__)
+    // _value may be an invalid value, which the clang sanitizer reports
+    // as undefined behavior, even though the value is acceptable in this
+    // context.
+    __attribute__((no_sanitize ("undefined")))
+#endif
 {
     unsigned char tmp = _value;
     Xdr::write <StreamIO> (os, tmp);

diff --git a/OpenEXR/IlmImf/ImfDeepScanLineInputFile.cpp b/OpenEXR/IlmImf/ImfDeepScanLineInputFile.cpp
@@ -721,10 +721,12 @@ LineBufferTask::execute ()
 
                     int width = (_ifd->maxX - _ifd->minX + 1);
 
+                    ptrdiff_t base = reinterpret_cast<ptrdiff_t>(&_ifd->sampleCount[0][0]);
+                    base -= sizeof(unsigned int)*_ifd->minX;
+                    base -= sizeof(unsigned int)*static_cast<ptrdiff_t>(_ifd->minY) * static_cast<ptrdiff_t>(width);
+
                     copyIntoDeepFrameBuffer (readPtr, slice.base,
-                                             (char*) (&_ifd->sampleCount[0][0]
-                                                      - _ifd->minX
-                                                      - _ifd->minY * width),
+                                             reinterpret_cast<char*>(base),
                                              sizeof(unsigned int) * 1,
                                              sizeof(unsigned int) * width,
                                              y, _ifd->minX, _ifd->maxX,

diff --git a/OpenEXR/IlmImf/ImfDeepTiledInputFile.cpp b/OpenEXR/IlmImf/ImfDeepTiledInputFile.cpp
@@ -990,8 +990,8 @@ DeepTiledInputFile::initialize ()
     for (size_t i = 0; i < _data->tileBuffers.size(); i++)
         _data->tileBuffers[i] = new TileBuffer ();
 
-    _data->maxSampleCountTableSize = _data->tileDesc.ySize *
-                                     _data->tileDesc.xSize *
+    _data->maxSampleCountTableSize = static_cast<size_t>(_data->tileDesc.ySize) *
+                                     static_cast<size_t>(_data->tileDesc.xSize) *
                                      sizeof(int);
 
     _data->sampleCountTableBuffer.resizeErase(_data->maxSampleCountTableSize);

diff --git a/OpenEXR/IlmImf/ImfDwaCompressor.cpp b/OpenEXR/IlmImf/ImfDwaCompressor.cpp
@@ -2535,7 +2535,7 @@ DwaCompressor::uncompress
 
     if (acCompressedSize > 0)
     {
-        if (totalAcUncompressedCount*sizeof(unsigned short) > _packedAcBufferSize)
+        if ( !_packedAcBuffer || totalAcUncompressedCount*sizeof(unsigned short) > _packedAcBufferSize)
         {
             throw IEX_NAMESPACE::InputExc("Error uncompressing DWA data"
                                 "(corrupt header).");
@@ -2681,6 +2681,10 @@ DwaCompressor::uncompress
         int gChan = _cscSets[csc].idx[1];    
         int bChan = _cscSets[csc].idx[2];    
 
+        if (_channelData[rChan].compression != LOSSY_DCT || _channelData[gChan].compression != LOSSY_DCT || _channelData[bChan].compression != LOSSY_DCT)
+        {
+            throw IEX_NAMESPACE::BaseExc("Bad DWA compression type detected");
+        }
 
         LossyDctDecoderCsc decoder
             (rowPtrs[rChan],

diff --git a/OpenEXR/IlmImf/ImfEnvmapAttribute.cpp b/OpenEXR/IlmImf/ImfEnvmapAttribute.cpp
@@ -57,6 +57,12 @@ EnvmapAttribute::staticTypeName ()
 template <>
 void
 EnvmapAttribute::writeValueTo (OPENEXR_IMF_INTERNAL_NAMESPACE::OStream &os, int version) const
+#if defined (__clang__)
+    // _value may be an invalid value, which the clang sanitizer reports
+    // as undefined behavior, even though the value is acceptable in this
+    // context.
+    __attribute__((no_sanitize ("undefined")))
+#endif
 {
     unsigned char tmp = _value;
     Xdr::write <StreamIO> (os, tmp);

diff --git a/OpenEXR/IlmImf/ImfFastHuf.cpp b/OpenEXR/IlmImf/ImfFastHuf.cpp
@@ -127,7 +127,7 @@ FastHufDecoder::FastHufDecoder
 
     for (Int64 symbol = static_cast<Int64>(minSymbol); symbol <= static_cast<Int64>(maxSymbol); symbol++)
     {
-        if (currByte - table > numBytes)
+        if (currByte - table >= numBytes)
         {
             throw IEX_NAMESPACE::InputExc ("Error decoding Huffman table "
                                            "(Truncated table data).");
@@ -144,7 +144,7 @@ FastHufDecoder::FastHufDecoder
 
         if (codeLen == (Int64) LONG_ZEROCODE_RUN)
         {
-            if (currByte - table > numBytes)
+            if (currByte - table >= numBytes)
             {
                 throw IEX_NAMESPACE::InputExc ("Error decoding Huffman table "
                                                "(Truncated table data).");
@@ -205,7 +205,7 @@ FastHufDecoder::FastHufDecoder
         for (int l = _minCodeLength; l <= _maxCodeLength; ++l)
         {
             countTmp[l] = (double)codeCount[l] * 
-                          (double)(2 << (_maxCodeLength-l));
+                          (double)(2ll << (_maxCodeLength-l));
         }
 
         for (int l = _minCodeLength; l <= _maxCodeLength; ++l)
@@ -215,7 +215,7 @@ FastHufDecoder::FastHufDecoder
             for (int k =l + 1; k <= _maxCodeLength; ++k)
                 tmp += countTmp[k];
 
-            tmp /= (double)(2 << (_maxCodeLength - l));
+            tmp /= (double)(2ll << (_maxCodeLength - l));
 
             base[l] = (Int64)ceil (tmp);
         }
@@ -538,18 +538,24 @@ FastHufDecoder::refill
 
         buffer |= bufferBack >> (64 - numBits);
     }
-
-    bufferBack         = bufferBack << numBits;
-    bufferBackNumBits -= numBits;
 
-    // 
-    // We can have cases where the previous shift of bufferBack is << 64 - 
-    // in which case no shift occurs. The bit count math still works though,
-    // so if we don't have any bits left, zero out bufferBack.
+
+    //
+    // We can have cases where the previous shift of bufferBack is << 64 -
+    // this is an undefined operation but tends to create just zeroes.
+    // so if we won't have any bits left, zero out bufferBack insetad of computing the shift
     //
 
-    if (bufferBackNumBits == 0)
+    if (bufferBackNumBits <= numBits)
+    {
         bufferBack = 0;
+    }else
+    {
+        bufferBack = bufferBack << numBits;
+    }
+    bufferBackNumBits -= numBits;
+
+
 }
 
 //

diff --git a/OpenEXR/IlmImf/ImfHuf.cpp b/OpenEXR/IlmImf/ImfHuf.cpp
@@ -910,6 +910,11 @@ hufDecode
 		//
 
 		lc -= pl.len;
+
+		if ( lc < 0 )
+		{
+			invalidCode(); // code length too long
+		}
 		getCode (pl.lit, rlc, c, lc, in, out, outb, oe);
 	    }
 	    else
@@ -967,6 +972,10 @@ hufDecode
 	if (pl.len)
 	{
 	    lc -= pl.len;
+            if ( lc < 0 )
+            {
+   	        invalidCode(); // code length too long
+            }
 	    getCode (pl.lit, rlc, c, lc, in, out, outb, oe);
 	}
 	else
@@ -1093,7 +1102,9 @@ hufUncompress (const char compressed[],
 
     const char *ptr = compressed + 20;
 
-    if ( ptr + (nBits+7 )/8 > compressed+nCompressed)
+    uint64_t nBytes = (static_cast<uint64_t>(nBits)+7) / 8 ;
+
+    if ( ptr + nBytes > compressed+nCompressed)
     {
         notEnoughData();
         return;