From fdfedfc07701a1db96b8217321b09bd19808ac24 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Tue, 25 Jan 2011 11:23:42 +0000 Subject: [PATCH] Fixed heap corruption when reading a corrupted RLE TGA image. There were no boundary checks in place to verify an indicated repeat of pixels would still be inside the image's data. Added these checks and a unit test making sure these kind of TGAs now fail to load. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@66758 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- src/common/imagtga.cpp | 10 ++++++++++ tests/image/image.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/common/imagtga.cpp b/src/common/imagtga.cpp index 8f9737ca4e..3d412ddeed 100644 --- a/src/common/imagtga.cpp +++ b/src/common/imagtga.cpp @@ -131,6 +131,11 @@ int DecodeRLE(unsigned char* imageData, unsigned long imageSize, index += current * pixelSize; + if (index >= imageSize) + { + return wxTGA_IOERR; + } + // Repeat the pixel length times. if ( !stream.Read(buf, pixelSize) ) return wxTGA_IOERR; @@ -151,6 +156,11 @@ int DecodeRLE(unsigned char* imageData, unsigned long imageSize, index += length; + if (index >= imageSize) + { + return wxTGA_IOERR; + } + // Write the next length pixels directly to the image data. if ( !stream.Read(imageData, length) ) return wxTGA_IOERR; diff --git a/tests/image/image.cpp b/tests/image/image.cpp index 47114b52e8..241001fcaf 100644 --- a/tests/image/image.cpp +++ b/tests/image/image.cpp @@ -72,6 +72,7 @@ private: CPPUNIT_TEST( CompareLoadedImage ); CPPUNIT_TEST( CompareSavedImage ); CPPUNIT_TEST( SaveAnimatedGIF ); + CPPUNIT_TEST( ReadCorruptedTGA ); CPPUNIT_TEST_SUITE_END(); void LoadFromSocketStream(); @@ -81,6 +82,7 @@ private: void CompareLoadedImage(); void CompareSavedImage(); void SaveAnimatedGIF(); + void ReadCorruptedTGA(); DECLARE_NO_COPY_CLASS(ImageTestCase) }; @@ -1133,6 +1135,42 @@ void ImageTestCase::SaveAnimatedGIF() #endif // #if wxUSE_PALETTE } +void ImageTestCase::ReadCorruptedTGA() +{ + static unsigned char corruptTGA[18+1+3] = + { + 0, + 0, + 10, // RLE compressed image. + 0, 0, + 0, 0, + 0, + 0, 0, + 0, 0, + 1, 0, // Width is 1. + 1, 0, // Height is 1. + 24, // Bits per pixel. + 0, + + 0xff, // Run length (repeat next pixel 127+1 times). + 0xff, 0xff, 0xff // One 24-bit pixel. + }; + + wxMemoryInputStream memIn(corruptTGA, WXSIZEOF(corruptTGA)); + CPPUNIT_ASSERT(memIn.IsOk()); + + wxImage tgaImage; + CPPUNIT_ASSERT( !tgaImage.LoadFile(memIn) ); + + + /* + Instead of repeating a pixel 127+1 times, now tell it there will + follow 127+1 uncompressed pixels (while we only should have 1 in total). + */ + corruptTGA[18] = 0x7f; + CPPUNIT_ASSERT( !tgaImage.LoadFile(memIn) ); +} + #endif //wxUSE_IMAGE -- 2.49.0