Fixed buffer overflow when saving certain images in the Windows icon format.

When an image did not have a width with a multiple of 4 the calculations for the number of padding bytes (to get a scan line DWORD aligned) would be wrong. This caused a buffer overrun when saving the 1 bits per pixel mask.

Fixes #12937.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@67296 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/src/common/imagbmp.cpp b/src/common/imagbmp.cpp
index bc6f47c85841979209d41b42b59e2d9b4d6782a0..f691b233e63a18e23ff5823edbf10a184cb3830f 100644 (file)
--- a/src/common/imagbmp.cpp
+++ b/src/common/imagbmp.cpp
@@ -146,8 +146,8 @@ bool wxBMPHandler::SaveDib(wxImage *image,
     }
 
     unsigned width = image->GetWidth();
     }
 
     unsigned width = image->GetWidth();

-    unsigned row_padding = (4 - int(width*bpp/8.0) % 4) % 4; // # bytes to pad to dword
-    unsigned row_width = int(width * bpp/8.0) + row_padding; // # of bytes per row
+    unsigned row_padding = (4 - ((width * bpp + 7) / 8) % 4) % 4; // # bytes to pad to dword
+    unsigned row_width = (width * bpp + 7) / 8 + row_padding; // # of bytes per row

 
     struct
     {
 
     struct
     {

diff --git a/tests/image/image.cpp b/tests/image/image.cpp
index b1205843a73235807277f77e04ae8e3a832665d6..810b56d069243e756f6be792a412a9e6cce01fd1 100644 (file)
--- a/tests/image/image.cpp
+++ b/tests/image/image.cpp
@@ -75,6 +75,7 @@ private:
         CPPUNIT_TEST( SaveAnimatedGIF );
         CPPUNIT_TEST( ReadCorruptedTGA );
         CPPUNIT_TEST( GIFComment );
         CPPUNIT_TEST( SaveAnimatedGIF );
         CPPUNIT_TEST( ReadCorruptedTGA );
         CPPUNIT_TEST( GIFComment );

+        CPPUNIT_TEST( DibPadding );

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


@@ -87,6 +88,7 @@ private:
     void SaveAnimatedGIF();
     void ReadCorruptedTGA();
     void GIFComment();
     void SaveAnimatedGIF();
     void ReadCorruptedTGA();
     void GIFComment();

+    void DibPadding();

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


@@ -900,6 +902,7 @@ void CompareImage(const wxImageHandler& handler, const wxImage& image,
     if ( testPalette
         && ( !(type == wxBITMAP_TYPE_BMP
                 || type == wxBITMAP_TYPE_GIF
     if ( testPalette
         && ( !(type == wxBITMAP_TYPE_BMP
                 || type == wxBITMAP_TYPE_GIF

+                || type == wxBITMAP_TYPE_ICO

                 || type == wxBITMAP_TYPE_PNG)
             || type == wxBITMAP_TYPE_XPM) )
     {
                 || type == wxBITMAP_TYPE_PNG)
             || type == wxBITMAP_TYPE_XPM) )
     {


@@ -1251,6 +1254,23 @@ void ImageTestCase::GIFComment()
     }
 }
 
     }
 }
 

+void ImageTestCase::DibPadding()
+{
+    /*
+    There used to be an error with calculating the DWORD aligned scan line
+    pitch for a BMP/ICO resulting in buffer overwrites (with at least MSVC9
+    Debug this gave a heap corruption assertion when saving the mask of
+    an ICO). Test for it here.
+    */
+    wxImage image("horse.gif");
+    CPPUNIT_ASSERT( image.IsOk() );
+
+    image = image.Scale(99, 99);
+
+    CompareImage(*wxImage::FindHandler(wxBITMAP_TYPE_ICO),
+        image, wxIMAGE_HAVE_PALETTE);
+}
+

 #endif //wxUSE_IMAGE
 
 
 #endif //wxUSE_IMAGE