[Git][wxwidgets/wxwidgets][master] 3 commits: Reject BMP RLE absolute runs that overrun the current scanline

1 view
Skip to first unread message

Vadim Zeitlin (@_VZ_)

unread,
May 22, 2026, 11:15:24 AMMay 22
to wx-commi...@googlegroups.com

Vadim Zeitlin pushed to branch master at wxWidgets / wxWidgets

Commits:

  • d1d6605e
    by dxbjavid at 2026-05-22T16:37:21+02:00 
    Reject BMP RLE absolute runs that overrun the current scanline

In src/common/imagbmp.cpp::LoadBMPData() the absolute-mode branches of
both the BI_RLE4 (around line 763) and BI_RLE8 (around line 870) decode
loops increment 'column' for each pixel without checking it against
'width'. The image buffer is sized width * height * 3 and 'poffset' is
computed as line * width * 3 + column * 3, so an absolute escape with
a count larger than (width - column) keeps writing through the rest of
the row and into adjacent rows or past the end of the buffer entirely
on the last decoded scanline.

The neighbouring encoded-mode branches at lines 798 and 896 already use
"&& column < width" to clamp runs to the row width, and the delta-mode
branch at lines 758 and 863 rejects out-of-range row offsets, so the
absolute branches are the only RLE paths left without bounds checking.

Reject the file with return false when the absolute run would extend
past the right edge of the current row, matching the existing
"return false on malformed input" pattern in the same function.

Add a regression test that loads a 4x4 8bpp RLE BMP with an absolute
escape claiming 100 pixels on the first row and expects the loader to
fail rather than write past the image buffer.

Closes #26496.
  • d22a91e9
    by jmestwa-coder at 2026-05-22T16:44:00+02:00 
    Validate IFF BMHD fields to prevent pixel-buffer overflow

wxIFFDecoder::ReadIFF() in src/common/imagiff.cpp parses the BMHD
chunk without bounding the width, height or bitplane count. The
subsequent pixel buffer is allocated with

    m_image->p = new byte[bmhd_width * bmhd_height * 3];

using signed-int multiplication. With bmhd_width = 21849 and
bmhd_height = 65535 (both legal 16-bit BMHD values) the product
4,295,622,645 overflows int and wraps down to 655,349, so only
~640 KiB is actually allocated. The BODY decode loop then writes
3 * bmhd_width bytes per row, so a BODY chunk supplying just 10
lineskips of zeros (lineskip = 2732 for this width, total 27,320
bytes) is enough to overrun the allocation. A bmhd_bitplanes or
bmhd_width of zero also makes lineskip * bmhd_bitplanes zero and
causes a divide-by-zero in the height computation a few lines
later.

Reject malformed BMHD chunks at parse time: require positive
width, height and bitplane count, cap the bitplane count at 32
(the largest format the decoder handles is 24-bit ILBM), and cap
bmhd_width * bmhd_height so that the product multiplied by 3
stays within INT_MAX. This makes the existing int-typed buffer
size computation safe and keeps the patch local to the BMHD
parsing branch.

A unit test in tests/image/image.cpp builds the malformed IFF
described above in memory, registers the IFF handler and asserts
that LoadFile() rejects it; without the fix the test triggers a
heap-buffer-overflow during BODY decoding.

Closes #26497.
  • 2d8898b0
    by Vadim Zeitlin at 2026-05-22T17:11:49+02:00 
    Use wxDEPRECATED_BUT_USED_INTERNALLY_MSG() in wxPG headers

It shouldn't be necessary to use this macro instead of the previously
used wxDEPRECATED_MSG() here because none of the functions is actually
used by wxWidgets, but MSVS 2026 somehow thinks they are and gives
warnings about it, so avoid the warning to fix the GitHub Actions CI job
using this compiler.

5 changed files:

Changes:

  • include/wx/propgrid/advprops.h
    ... ... @@ -210,7 +210,7 @@ public:
    210 210 
         virtual int GetCustomColourIndex() const;
    211 211
    212 212 
     #if WXWIN_COMPATIBILITY_3_2
    213 
    -    wxDEPRECATED_MSG("use ValueToString with 'flags' argument as wxPGPropValFormatFlags")
    213 
    +    wxDEPRECATED_BUT_USED_INTERNALLY_MSG("use ValueToString with 'flags' argument as wxPGPropValFormatFlags")
    214 214 
         virtual wxString ValueToString(wxVariant& value, int flags) const override
    215 215 
         {
    216 216 
             m_oldValueToStringCalled = true;

  • include/wx/propgrid/props.h
    ... ... @@ -506,7 +506,7 @@ public:
    506 506
    507 507 
         virtual void OnSetValue() override;
    508 508 
     #if WXWIN_COMPATIBILITY_3_2
    509 
    -    wxDEPRECATED_MSG("use ValueToString with 'flags' argument as wxPGPropValFormatFlags")
    509 
    +    wxDEPRECATED_BUT_USED_INTERNALLY_MSG("use ValueToString with 'flags' argument as wxPGPropValFormatFlags")
    510 510 
         virtual wxString ValueToString(wxVariant& value, int flags) const override
    511 511 
         {
    512 512 
             m_oldValueToStringCalled = true;
    ... ... @@ -515,7 +515,7 @@ public:
    515 515 
     #endif // WXWIN_COMPATIBILITY_3_2
    516 516 
         virtual wxString ValueToString(wxVariant& value, wxPGPropValFormatFlags flags = wxPGPropValFormatFlags::Null) const override;
    517 517 
     #if WXWIN_COMPATIBILITY_3_2
    518 
    -    wxDEPRECATED_MSG("use StringToValue with 'flags' argument as wxPGPropValFormatFlags")
    518 
    +    wxDEPRECATED_BUT_USED_INTERNALLY_MSG("use StringToValue with 'flags' argument as wxPGPropValFormatFlags")
    519 519 
         virtual bool StringToValue(wxVariant& variant, const wxString& text,
    520 520 
                                    int flags) const override
    521 521 
         {
    ... ... @@ -531,7 +531,7 @@ public:
    531 531 
         // If wxPGPropValFormatFlags::FullValue is not set in flags, then the value is interpreted
    532 532 
         // as index to choices list. Otherwise, it is actual value.
    533 533 
     #if WXWIN_COMPATIBILITY_3_2
    534 
    -    wxDEPRECATED_MSG("use IntToValue with 'flags' argument as wxPGPropValFormatFlags")
    534 
    +    wxDEPRECATED_BUT_USED_INTERNALLY_MSG("use IntToValue with 'flags' argument as wxPGPropValFormatFlags")
    535 535 
         virtual bool IntToValue(wxVariant& variant, int number, int flags) const override
    536 536 
         {
    537 537 
             m_oldIntToValueCalled = true;

  • src/common/imagbmp.cpp
    ... ... @@ -763,6 +763,13 @@ bool LoadBMPData(wxImage * image, const BMPDesc& desc,
    763 763 
                                 {
    764 764 
                                     // absolute mode (pixels not runs)
    765 765 
                                     int absolute = aByte;
    766 
    +                                // RLE runs do not span scanlines; reject a
    767 
    +                                // file whose absolute run would advance past
    768 
    +                                // the right edge of the row and write into
    769 
    +                                // adjacent rows or past the end of the image
    770 
    +                                // buffer.
    771 
    +                                if ( column + absolute > width )
    772 
    +                                    return false;
    766 773 
                                     wxUint8 nibble[2] ;
    767 774 
                                     int readBytes = 0 ;
    768 775 
                                     for (int k = 0; k < absolute; k++)
    ... ... @@ -868,6 +875,13 @@ bool LoadBMPData(wxImage * image, const BMPDesc& desc,
    868 875 
                                 {
    869 876 
                                     // absolute mode (pixels not runs)
    870 877 
                                     int absolute = aByte;
    878 
    +                                // RLE runs do not span scanlines; reject a
    879 
    +                                // file whose absolute run would advance past
    880 
    +                                // the right edge of the row and write into
    881 
    +                                // adjacent rows or past the end of the image
    882 
    +                                // buffer.
    883 
    +                                if ( column + absolute > width )
    884 
    +                                    return false;
    871 885 
                                     for (int k = 0; k < absolute; k++)
    872 886 
                                     {
    873 887 
                                         aByte = stream.GetC();

  • src/common/imagiff.cpp
    ... ... @@ -29,6 +29,7 @@
    29 29 
         #include "wx/palette.h"
    30 30 
     #endif // wxUSE_PALETTE
    31 31
    32 
    +#include <limits.h>     // for INT_MAX
    32 33 
     #include <stdlib.h>
    33 34 
     #include <string.h>
    34 35
    ... ... @@ -404,6 +405,23 @@ int wxIFFDecoder::ReadIFF()
    404 405 
             // bmhd_masking  = *(dataptr + 8 + 9); -- unused currently
    405 406 
             bmhd_compression = *(dataptr + 8 + 10);     // get compression
    406 407 
             bmhd_transcol    = iff_getword(dataptr + 8 + 12);
    408 
    +
    409 
    +        // Reject malformed BMHD chunks: zero width/height or bitplanes
    410 
    +        // would later cause a divide-by-zero when computing the lineskip
    411 
    +        // and row count for the BODY chunk; oversized dimensions would
    412 
    +        // overflow the signed-int product used to size the pixel buffer
    413 
    +        // (new byte[bmhd_width * bmhd_height * 3] below), leaving an
    414 
    +        // undersized allocation behind for the BODY decode loop to
    415 
    +        // overrun. Cap bmhd_width * bmhd_height so that the same product
    416 
    +        // multiplied by 3 stays within INT_MAX.
    417 
    +        if (bmhd_width <= 0 || bmhd_height <= 0
    418 
    +                || bmhd_bitplanes <= 0 || bmhd_bitplanes > 32
    419 
    +                || static_cast<wxUint64>(bmhd_width)
    420 
    +                    * static_cast<wxUint64>(bmhd_height)
    421 
    +                        > static_cast<wxUint64>(INT_MAX) / 3) {
    422 
    +            break;
    423 
    +        }
    424 
    +
    407 425 
             BMHDok = true;                              // got BMHD
    408 426 
             dataptr += 8 + chunkLen;                    // to next chunk
    409 427 
         }

  • tests/image/image.cpp
    ... ... @@ -1387,6 +1387,47 @@ TEST_CASE_METHOD(ImageHandlersInit, "wxImage::BadIFF", "[image][iff][error]")
    1387 1387 
         REQUIRE( !img.LoadFile(mis, wxBITMAP_TYPE_IFF) );
    1388 1388 
     }
    1389 1389
    1390 
    +TEST_CASE_METHOD(ImageHandlersInit, "wxImage::BadIFFBmhdOverflow",
    1391 
    +                 "[image][iff][error]")
    1392 
    +{
    1393 
    +    // BMHD width = 21849, height = 65535 gives bmhd_width * bmhd_height * 3
    1394 
    +    // = 4 295 622 645, which overflows the 32-bit signed int product used by
    1395 
    +    // wxIFFDecoder::ReadIFF() to size the pixel buffer and wraps down to
    1396 
    +    // 655 349. The subsequent BODY decode loop writes 3 * bmhd_width bytes
    1397 
    +    // per scanline, so a BODY chunk containing 10 lineskips' worth of zeros
    1398 
    +    // (lineskip = 2732 for this width, total 27320 bytes) is enough to
    1399 
    +    // overrun the undersized heap allocation. The fix rejects the file at
    1400 
    +    // BMHD validation time.
    1401 
    +    wxImage::AddHandler(new wxIFFHandler);
    1402 
    +
    1403 
    +    static const unsigned char header[] =
    1404 
    +    {
    1405 
    +        0x46,0x4f,0x52,0x4d,            // "FORM"
    1406 
    +        0x00,0x00,0x6a,0xe0,            // FORM length = 27360
    1407 
    +        0x49,0x4c,0x42,0x4d,            // "ILBM"
    1408 
    +        0x42,0x4d,0x48,0x44,            // "BMHD"
    1409 
    +        0x00,0x00,0x00,0x14,            // BMHD chunk length = 20
    1410 
    +        0x55,0x59,                      // width  = 21849
    1411 
    +        0xff,0xff,                      // height = 65535
    1412 
    +        0x00,0x00,0x00,0x00,            // x, y
    1413 
    +        0x01,                           // nPlanes
    1414 
    +        0x00,                           // masking
    1415 
    +        0x00,                           // compression (BI_RGB, uncompressed)
    1416 
    +        0x00,                           // pad
    1417 
    +        0x00,0x00,                      // transparentColor
    1418 
    +        0x00,0x00,                      // x/y aspect
    1419 
    +        0x00,0x00,0x00,0x00,            // page width / height
    1420 
    +        0x42,0x4f,0x44,0x59,            // "BODY"
    1421 
    +        0x00,0x00,0x6a,0xb8,            // BODY chunk length = 27320
    1422 
    +    };
    1423 
    +    std::vector<unsigned char> data(header, header + WXSIZEOF(header));
    1424 
    +    data.resize(data.size() + 27320, 0);
    1425 
    +
    1426 
    +    wxMemoryInputStream mis(data.data(), data.size());
    1427 
    +    wxImage img;
    1428 
    +    REQUIRE( !img.LoadFile(mis, wxBITMAP_TYPE_IFF) );
    1429 
    +}
    1430 
    +
    1390 1431 
     #endif // wxUSE_IFF
    1391 1432
    1392 1433 
     TEST_CASE_METHOD(ImageHandlersInit, "wxImage::DibPadding", "[image]")

Reply all
Reply to author
Forward
0 new messages