Add additional input validation in an attempt to resolve issue Academ…

…ySoftwareFoundation#232

OpenEXR/IlmImf/ImfDwaCompressor.cpp

@@ -2386,7 +2386,12 @@ DwaCompressor::uncompress
 
     const char *dataPtr            = inPtr + NUM_SIZES_SINGLE * sizeof(Int64);
 
-    if (inSize < headerSize + compressedSize) 
+    /* Both the sum and individual sizes are checked in case of overflow. */
+    if (inSize < (headerSize + compressedSize) ||
+        inSize < unknownCompressedSize ||
+        inSize < acCompressedSize ||
+        inSize < dcCompressedSize ||
+        inSize < rleCompressedSize)
     {
         throw Iex::InputExc("Error uncompressing DWA data"
                             "(truncated file).");

OpenEXR/IlmImf/ImfHuf.cpp

@@ -822,7 +822,7 @@ hufEncode				// return: output size (in bits)
 }
 
 
-#define getCode(po, rlc, c, lc, in, out, oe)	\
+#define getCode(po, rlc, c, lc, in, out, ob, oe)\
 {						\
     if (po == rlc)				\
     {						\
@@ -835,6 +835,8 @@ hufEncode				// return: output size (in bits)
 						\
 	if (out + cs > oe)			\
 	    tooMuchData();			\
+	else if (out - 1 < ob)			\
+	    notEnoughData();			\
 						\
 	unsigned short s = out[-1];		\
 						\
@@ -895,7 +897,7 @@ hufDecode
 		//
 
 		lc -= pl.len;
-		getCode (pl.lit, rlc, c, lc, in, out, oe);
+		getCode (pl.lit, rlc, c, lc, in, out, outb, oe);
 	    }
 	    else
 	    {
@@ -925,7 +927,7 @@ hufDecode
 			    //
 
 			    lc -= l;
-			    getCode (pl.p[j], rlc, c, lc, in, out, oe);
+			    getCode (pl.p[j], rlc, c, lc, in, out, outb, oe);
 			    break;
 			}
 		    }
@@ -952,7 +954,7 @@ hufDecode
 	if (pl.len)
 	{
 	    lc -= pl.len;
-	    getCode (pl.lit, rlc, c, lc, in, out, oe);
+	    getCode (pl.lit, rlc, c, lc, in, out, outb, oe);
 	}
 	else
 	{

OpenEXR/IlmImf/ImfPizCompressor.cpp

@@ -573,6 +573,12 @@ PizCompressor::uncompress (const char *inPtr,
     int length;
     Xdr::read <CharPtrIO> (inPtr, length);
 
+    if (length > inSize)
+    {
+	throw InputExc ("Error in header for PIZ-compressed data "
+			"(invalid array length).");
+    }
+
     hufUncompress (inPtr, length, _tmpBuffer, tmpBufferEnd - _tmpBuffer);
 
     //

OpenEXR/IlmImfTest/CMakeLists.txt

@@ -32,6 +32,7 @@ ADD_EXECUTABLE ( IlmImfTest
   testLineOrder.cpp
   testLut.cpp
   testMagic.cpp
+  testMalformedImages.cpp
   testMultiPartApi.cpp
   testMultiPartFileMixingBasic.cpp
   testMultiPartSharedAttributes.cpp

OpenEXR/IlmImfTest/Makefile.am

@@ -5,12 +5,14 @@ check_PROGRAMS = IlmImfTest
 IlmImfTest_SOURCES = main.cpp tmpDir.h testAttributes.cpp testChannels.cpp \
 		     testCompression.cpp testCopyPixels.cpp \
 		     testCustomAttributes.cpp testHuf.cpp testLineOrder.cpp \
+		     testMalformedImages.cpp \
 		     testLut.cpp testRgba.cpp testRgbaThreading.cpp \
 		     testSampleImages.cpp testSharedFrameBuffer.cpp \
 		     testWav.cpp testXdr.cpp testAttributes.h testChannels.h \
 		     testCompression.h testCopyPixels.h \
 		     testCustomAttributes.h testHuf.h testLineOrder.h \
 		     testLut.h testRgba.h testRgbaThreading.h \
+		     testMalformedImages.h \
 		     testSampleImages.h testWav.h testSharedFrameBuffer.h \
 		     testXdr.h testConversion.cpp testConversion.h \
 		     testNativeFormat.cpp testNativeFormat.h \

OpenEXR/IlmImfTest/testMalformedImages.cpp

@@ -0,0 +1,153 @@
+///////////////////////////////////////////////////////////////////////////
+//
+// Copyright (c) 2002-2012, 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.
+//
+///////////////////////////////////////////////////////////////////////////
+
+#include <ImfAcesFile.h>
+#include <ImfArray.h>
+#include <ImfRgbaFile.h>
+#include <IlmThread.h>
+#include <stdio.h>
+#include <assert.h>
+
+#ifndef ILM_IMF_TEST_IMAGEDIR
+    #define ILM_IMF_TEST_IMAGEDIR
+#endif
+
+
+using namespace OPENEXR_IMF_NAMESPACE;
+using namespace std;
+using namespace IMATH_NAMESPACE;
+
+
+namespace {
+
+void
+readImage (const char inFileName[])
+{
+	Array2D<Rgba> p;
+	Header h;
+	RgbaChannels ch;
+	Box2i dw;
+	int width;
+	int height;
+
+	{
+		cout << "Reading file " << inFileName << endl;
+
+		AcesInputFile in (inFileName);
+
+		h = in.header();
+		ch = in.channels();
+		dw = h.dataWindow();
+
+		width  = dw.max.x - dw.min.x + 1;
+		height = dw.max.y - dw.min.y + 1;
+		p.resizeErase (height, width);
+
+		in.setFrameBuffer (&p[0][0] - dw.min.x - dw.min.y * width, 1, width);
+		in.readPixels (dw.min.y, dw.max.y);
+	}
+}
+
+void
+readBadBoundsImage (const char fileName[])
+{
+    cout << "file " << fileName << " " << flush;
+
+    OPENEXR_IMF_NAMESPACE::RgbaInputFile in (fileName);
+
+    cout << "version " << in.version() << " " << flush;
+
+    const Box2i &dw = in.dataWindow();
+}
+
+} // namespace
+
+void
+testMalformedImages (const std::string&)
+{
+	try
+	{
+		// id:000012,sig:11,src:000328+001154,op:splice,rep:16
+		readImage (ILM_IMF_TEST_IMAGEDIR "comp_short_decode_piz.exr");
+		cerr << "Malformed Images : InputFile : incorrect input file passed\n";
+		assert (false);
+	}
+	catch (const IEX_NAMESPACE::BaseExc &e)
+	{
+		// expected behaviour
+	}
+
+	try
+	{
+		// id:000077,sig:11,src:002575,op:havoc,rep:4
+		readImage (ILM_IMF_TEST_IMAGEDIR "comp_invalid_unknown.exr");
+		cerr << "Malformed Images : InputFile : incorrect input file passed\n";
+		assert (false);
+	}
+	catch (const IEX_NAMESPACE::IoExc &e)
+	{
+		// expected behaviour
+	}
+
+	try
+	{
+		// id:000103,sig:11,src:002037+004745,op:splice,rep:2
+		readImage (ILM_IMF_TEST_IMAGEDIR "comp_early_eof_piz.exr");
+		cerr << "Malformed Images : InputFile : incorrect input file passed\n";
+		assert (false);
+	}
+	catch (const IEX_NAMESPACE::InputExc &e)
+	{
+		// expected behaviour
+	}
+
+	// The files below expose a bug in the test code (readImage which uses the
+	// logic taken from exr2aces) that calculates an invalid pointer for the
+	// framebuffer.  The dataWindow and displayWindow values used in these files
+	// seem valid based on a cursory reading of the OpenEXR specification. As
+	// such, the best we can do is ensure that parsing the basic header
+	// information doesn't cause any unexpected exceptions.
+
+	// id:000087,sig:11,src:000562+000300,op:splice,rep:2
+	readBadBoundsImage (ILM_IMF_TEST_IMAGEDIR "comp_bad_pos_bounds_piz.exr");
+
+	// id:000104,sig:11,src:001329+000334,op:splice,rep:2
+	readBadBoundsImage (ILM_IMF_TEST_IMAGEDIR "comp_bad_pos_bounds_pxr24.exr");
+
+	// id:000131,sig:11,src:000514+002831,op:splice,rep:16
+	readBadBoundsImage (ILM_IMF_TEST_IMAGEDIR "comp_bad_neg_bounds_pxr24.exr");
+
+	// id:000132,sig:11,src:000895,op:havoc,rep:32
+	readBadBoundsImage (ILM_IMF_TEST_IMAGEDIR "comp_bad_bounds_piz.exr");
+}

OpenEXR/IlmImfTest/testMalformedImages.h

@@ -0,0 +1,37 @@
+///////////////////////////////////////////////////////////////////////////
+//
+// Copyright (c) 2002-2012, 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.
+//
+///////////////////////////////////////////////////////////////////////////
+
+#include <string>
+
+void testMalformedImages (const std::string &tempDir);