From b04c1ace47a12aea0fcda306bae91d0fb1526e81 Mon Sep 17 00:00:00 2001 From: Paul Cornett Date: Sat, 27 Nov 2021 17:14:28 -0800 Subject: [PATCH] Avoid integer overflow when computing image data size in wxImage::Create() See #19326 Co-Authored-By: David Costanzo --- build/cmake/tests/gui/CMakeLists.txt | 2 + src/common/image.cpp | 21 +- tests/Makefile.in | 2 +- tests/image/image.cpp | 71 +- tests/image/width-times-height-overflow.bmp | Bin 0 -> 153666 bytes tests/image/width_height_32_bit_overflow.pgm | 2395 ++++++++++++++++++ tests/makefile.gcc | 2 +- tests/makefile.vc | 2 +- tests/test.bkl | 3 + 9 files changed, 2473 insertions(+), 25 deletions(-) create mode 100644 tests/image/width-times-height-overflow.bmp create mode 100644 tests/image/width_height_32_bit_overflow.pgm diff --git a/build/cmake/tests/gui/CMakeLists.txt b/build/cmake/tests/gui/CMakeLists.txt index e2a988ece1..7ff31daf37 100644 --- a/build/cmake/tests/gui/CMakeLists.txt +++ b/build/cmake/tests/gui/CMakeLists.txt @@ -191,6 +191,8 @@ set(TEST_GUI_DATA image/toucan_dis_240.png image/toucan_grey.png image/toucan_mono_255_255_255.png + image/width-times-height-overflow.bmp + image/width_height_32_bit_overflow.pgm intl/ja/internat.mo intl/ja/internat.po ) diff --git a/src/common/image.cpp b/src/common/image.cpp index 7df6d86fab..9bb7ca2461 100644 --- a/src/common/image.cpp +++ b/src/common/image.cpp @@ -156,15 +156,22 @@ bool wxImage::Create( int width, int height, bool clear ) { UnRef(); - m_refData = new wxImageRefData(); - - M_IMGDATA->m_data = (unsigned char *) malloc( width*height*3 ); - if (!M_IMGDATA->m_data) - { - UnRef(); + if (width <= 0 || height <= 0) return false; - } + const unsigned long long size = (unsigned long long)width * height * 3; + + // In theory, 64-bit architectures could handle larger sizes, + // but wxImage code is riddled with int-based arithmetic which will overflow + if (size > INT_MAX) + return false; + + unsigned char* p = (unsigned char*)malloc(size_t(size)); + if (p == NULL) + return false; + + m_refData = new wxImageRefData; + M_IMGDATA->m_data = p; M_IMGDATA->m_width = width; M_IMGDATA->m_height = height; M_IMGDATA->m_ok = true; diff --git a/tests/Makefile.in b/tests/Makefile.in index 61f8a5ef65..747e4c5f18 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -565,7 +565,7 @@ data: data-images: @mkdir -p image - @for f in 8bpp-colorsused-large.bmp 8bpp-colorsused-negative.bmp rle4-delta-320x240.bmp rle8-delta-320x240.bmp rle8-delta-320x240-expected.bmp horse_grey.bmp horse_grey_flipped.bmp horse_rle4.bmp horse_rle4_flipped.bmp horse_rle8.bmp horse_rle8_flipped.bmp horse_bicubic_50x50.png horse_bicubic_100x100.png horse_bicubic_150x150.png horse_bicubic_300x300.png horse_bilinear_50x50.png horse_bilinear_100x100.png horse_bilinear_150x150.png horse_bilinear_300x300.png horse_box_average_50x50.png horse_box_average_100x100.png horse_box_average_150x150.png horse_box_average_300x300.png cross_bicubic_256x256.png cross_bilinear_256x256.png cross_box_average_256x256.png cross_nearest_neighb_256x256.png paste_input_background.png paste_input_black.png paste_input_overlay_transparent_border_opaque_square.png paste_input_overlay_transparent_border_semitransparent_circle.png paste_input_overlay_transparent_border_semitransparent_square.png paste_result_background_plus_circle_plus_square.png paste_result_background_plus_overlay_transparent_border_opaque_square.png paste_result_background_plus_overlay_transparent_border_semitransparent_square.png paste_result_no_background_square_over_circle.png wx.png wx.ico toucan.png toucan_hue_0.538.png toucan_sat_-0.41.png toucan_bright_-0.259.png toucan_hsv_0.538_-0.41_-0.259.png toucan_light_46.png toucan_dis_240.png toucan_grey.png toucan_mono_255_255_255.png; do \ + @for f in 8bpp-colorsused-large.bmp 8bpp-colorsused-negative.bmp rle4-delta-320x240.bmp rle8-delta-320x240.bmp rle8-delta-320x240-expected.bmp horse_grey.bmp horse_grey_flipped.bmp horse_rle4.bmp horse_rle4_flipped.bmp horse_rle8.bmp horse_rle8_flipped.bmp horse_bicubic_50x50.png horse_bicubic_100x100.png horse_bicubic_150x150.png horse_bicubic_300x300.png horse_bilinear_50x50.png horse_bilinear_100x100.png horse_bilinear_150x150.png horse_bilinear_300x300.png horse_box_average_50x50.png horse_box_average_100x100.png horse_box_average_150x150.png horse_box_average_300x300.png cross_bicubic_256x256.png cross_bilinear_256x256.png cross_box_average_256x256.png cross_nearest_neighb_256x256.png paste_input_background.png paste_input_black.png paste_input_overlay_transparent_border_opaque_square.png paste_input_overlay_transparent_border_semitransparent_circle.png paste_input_overlay_transparent_border_semitransparent_square.png paste_result_background_plus_circle_plus_square.png paste_result_background_plus_overlay_transparent_border_opaque_square.png paste_result_background_plus_overlay_transparent_border_semitransparent_square.png paste_result_no_background_square_over_circle.png wx.png wx.ico toucan.png toucan_hue_0.538.png toucan_sat_-0.41.png toucan_bright_-0.259.png toucan_hsv_0.538_-0.41_-0.259.png toucan_light_46.png toucan_dis_240.png toucan_grey.png toucan_mono_255_255_255.png width-times-height-overflow.bmp width_height_32_bit_overflow.pgm; do \ if test ! -f image/$$f -a ! -d image/$$f ; \ then x=yep ; \ else x=`find $(srcdir)/image/$$f -newer image/$$f -print` ; \ diff --git a/tests/image/image.cpp b/tests/image/image.cpp index 009e94ca48..5eb923eb34 100644 --- a/tests/image/image.cpp +++ b/tests/image/image.cpp @@ -98,7 +98,6 @@ private: CPPUNIT_TEST( BMPFlippingAndRLECompression ); CPPUNIT_TEST( ScaleCompare ); CPPUNIT_TEST( CreateBitmapFromCursor ); - CPPUNIT_TEST( MalformedBMP ); CPPUNIT_TEST_SUITE_END(); void LoadFromSocketStream(); @@ -120,7 +119,6 @@ private: void BMPFlippingAndRLECompression(); void ScaleCompare(); void CreateBitmapFromCursor(); - void MalformedBMP(); wxDECLARE_NO_COPY_CLASS(ImageTestCase); }; @@ -1528,25 +1526,52 @@ void ImageTestCase::CreateBitmapFromCursor() } // This function assumes that the file is malformed in a way that it cannot -// be loaded. If the file is malformed such that wxImage can salvage part -// of it, then CompareBMPImage should be called instead. -static void LoadMalformedBMP(const wxString& file) +// be loaded but that doesn't fail a wxCHECK. +static void LoadMalformedImage(const wxString& file, wxBitmapType type) { - wxImage image(file); - WX_ASSERT_MESSAGE - ( - ("wxImage::isOk() returned true after loading \"%s\"", file), - !image.IsOk() - ); + // If the file doesn't exist, it's a test bug. + // (The product code would fail but for the wrong reason.) + INFO("Checking that image exists: " << file); + REQUIRE(wxFileExists(file)); + + // Load the image, expecting a failure. + wxImage image; + INFO("Loading malformed image: " << file); + REQUIRE(!image.LoadFile(file, type)); } -void ImageTestCase::MalformedBMP() +// This function assumes that the file is malformed in a way that wxImage +// fails a wxCHECK when trying to load it. +static void LoadMalformedImageWithException(const wxString& file, + wxBitmapType type) { - LoadMalformedBMP("image/8bpp-colorsused-negative.bmp"); - LoadMalformedBMP("image/8bpp-colorsused-large.bmp"); + // If the file doesn't exist, it's a test bug. + // (The product code would fail but for the wrong reason.) + INFO("Checking that image exists: " << file); + REQUIRE(wxFileExists(file)); + + wxImage image; + INFO("Loading malformed image: " << file); +#ifdef __WXDEBUG__ + REQUIRE_THROWS(image.LoadFile(file, type)); +#else + REQUIRE(!image.LoadFile(file, type)); +#endif } -#endif //wxUSE_IMAGE +TEST_CASE("wxImage::BMP", "[image][bmp]") +{ + SECTION("Load malformed bitmaps") + { + LoadMalformedImage("image/8bpp-colorsused-negative.bmp", + wxBITMAP_TYPE_BMP); + LoadMalformedImage("image/8bpp-colorsused-large.bmp", + wxBITMAP_TYPE_BMP); + + LoadMalformedImageWithException("image/width-times-height-overflow.bmp", + wxBITMAP_TYPE_BMP); + } +} TEST_CASE("wxImage::Paste", "[image][paste]") { @@ -2234,6 +2259,20 @@ TEST_CASE("wxImage::XPM", "[image][xpm]") CHECK( wxIcon(dummy_xpm).IsOk() ); } +TEST_CASE("wxImage::PNM", "[image][pnm]") +{ +#if wxUSE_PNM + wxImage::AddHandler(new wxPNMHandler); + + SECTION("width*height*3 overflow") + { + // wxImage should reject the file as malformed (wxTrac#19326) + LoadMalformedImageWithException("image/width_height_32_bit_overflow.pgm", + wxBITMAP_TYPE_PNM); + } +#endif +} + TEST_CASE("wxImage::ChangeColours", "[image]") { wxImage original; @@ -2298,3 +2337,5 @@ TEST_CASE("wxImage::SizeLimits", "[image]") /* TODO: add lots of more tests to wxImage functions */ + +#endif //wxUSE_IMAGE diff --git a/tests/image/width-times-height-overflow.bmp b/tests/image/width-times-height-overflow.bmp new file mode 100644 index 0000000000000000000000000000000000000000..24dd2cdfad70945f1a440060666d809a6b50eccb GIT binary patch literal 153666 zcmeIzF>)IP0EJ-zMOtop zfBNI+)8+B${QT|wy#Mq1{OR=T>6gnJ=lB0QA6%YaAD>UByAP++$FCfBJk|KX)H)@8{$BZgp!vXLV6v{PB1Ccsk@9N-nPtQjVh^Ro71(#kZe(ym9TG zoZ*i@{$6b0?hQZj|GWPlSGVK(_xNu7qwC-0{p0+@@p0z*`p<9IzZXm9@2RB2+vT0- zj-P(&e1<>%{^kAmIcLwE=kM;t)_(Mf+rR!@@2|)2i<|55<>)wmtnB-1-b4L#