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

diff --git a/src/common/imagtga.cpp b/src/common/imagtga.cpp
index 8f9737ca4e7c9e8a24a4ae1bcd3beb57883e32ba..3d412ddeed0dbda906eb425843d460b523396da8 100644 (file)
--- 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;
 
 
             index += current * pixelSize;
 

+            if (index >= imageSize)
+            {
+                return wxTGA_IOERR;
+            }
+

             // Repeat the pixel length times.
             if ( !stream.Read(buf, pixelSize) )
                 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;
 
 
             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;
             // 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 47114b52e84939284aa80c65809badc4989f560d..241001fcaf4bc0f75dfedddbf55df765230df31b 100644 (file)
--- 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( CompareLoadedImage );
         CPPUNIT_TEST( CompareSavedImage );
         CPPUNIT_TEST( SaveAnimatedGIF );

+        CPPUNIT_TEST( ReadCorruptedTGA );

     CPPUNIT_TEST_SUITE_END();
 
     void LoadFromSocketStream();
     CPPUNIT_TEST_SUITE_END();
 
     void LoadFromSocketStream();


@@ -81,6 +82,7 @@ private:
     void CompareLoadedImage();
     void CompareSavedImage();
     void SaveAnimatedGIF();
     void CompareLoadedImage();
     void CompareSavedImage();
     void SaveAnimatedGIF();

+    void ReadCorruptedTGA();

 
     DECLARE_NO_COPY_CLASS(ImageTestCase)
 };
 
     DECLARE_NO_COPY_CLASS(ImageTestCase)
 };


@@ -1133,6 +1135,42 @@ void ImageTestCase::SaveAnimatedGIF()
 #endif // #if wxUSE_PALETTE
 }
 
 #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
 
 
 #endif //wxUSE_IMAGE