[Git][wxwidgets/wxwidgets][master] 11 commits: Reject TGA files with non-zero colour map origin

1 view
Skip to first unread message

Vadim Zeitlin (@_VZ_)

unread,
May 25, 2026, 9:40:04 AM (13 days ago) May 25
to wx-commi...@googlegroups.com

Vadim Zeitlin pushed to branch master at wxWidgets / wxWidgets

Commits:

  • b2d7c29f
    by dxbjavid at 2026-05-23T20:55:04+02:00 
    Reject TGA files with non-zero colour map origin

ReadTGA() in src/common/imagtga.cpp allocates the palette buffer as
paletteLength * palEntrySize bytes (palette indices 0..paletteLength-1)
but the loop that fills it writes each entry at index paletteStart + i.
The paletteStart and paletteLength values come straight from the TGA
header (bytes 3-7 of the colour map specification) and aren't bounded
against each other. For any file with paletteStart > 0, the calls to
Palette_SetRGB()/Palette_SetRGBA() write past the end of the buffer:
e.g. paletteStart=100, paletteLength=10, palettebpp=24 allocates 30
bytes but writes at offsets 100..129. The subsequent
image->SetPalette(wxPalette((int) paletteLength, &palette[0], ...))
also reads from index 0 onward, so the rest of the loader was already
implicitly assuming paletteStart == 0.

Add an explicit early-return wxTGA_INVFORMAT in the colour-mapped
branch when paletteStart is non-zero, which is the assumption the
existing code makes anyway.

Closes #26493.
  • 313fbb08
    by Blake-Madden at 2026-05-25T06:08:59-04:00 
    Fix off-by-one buffer overflow in wxWebViewIE

wcscpy calling wasn't leaving space for the nul terminator
  • b6bfeaac
    by Blake-Madden at 2026-05-25T06:26:45-04:00 
    Fix typo in UuidToCForm format string and use bounded wxSnprintf
  • 4b720bb2
    by Blake-Madden at 2026-05-25T06:35:25-04:00 
    Replace wxStrcpy with wxStrlcpy
  • b1722286
    by Blake-Madden at 2026-05-25T06:47:33-04:00 
    Get charset from CHARFORMAT in wxTextCtrl instead of hardcoding ANSI_CHARSET
  • 0b87887a
    by Blake-Madden at 2026-05-25T07:12:50-04:00 
    Fix buffer size in characters, not bytes, for GetMenuItemInfo in dark mode menu handler

This is now it's done in mdi.cpp
  • 23775dfd
    by Blake-Madden at 2026-05-25T08:08:48-04:00 
    Use wxStrlcpy instead of wcscpy

This is being overly cautious since cchResult is verified above, but doesn't hurt
  • c8c1ca9b
    by dxbjavid at 2026-05-25T14:28:41+02:00 
    Stop reading past data end in truncated IFF BODY decode

Fix iff body truncation over-read in wxIFFDecoder::ReadIFF:
wxIFFDecoder::ReadIFF() sets chunkLen = dataend - dataptr on the
truncated BODY path, which is 8 too large because bodyptr lives 8 bytes
after dataptr. The non-RLE decode loop below then reads up to 8 bytes
past the end of databuf; clamp chunkLen to dataend - bodyptr instead.

Closes #26505.
  • 6069ea92
    by dxbjavid at 2026-05-25T14:33:32+02:00 
    Validate data chunk header room in wxSound::LoadWAV()

Don't read beyond the end of the buffer as could happen in case of a
44-byte WAV file with a 0-sized LIST chunk.

Closes #26506.
  • b29393b3
    by dxbjavid at 2026-05-25T14:40:16+02:00 
    Reject too-short ZIP64 extra field in wxZipEntry::LoadExtraInfo()

wxZipEntry::LoadExtraInfo() calls wxZipHeader::Read64() up to three
times on a wxZipHeader of length min(fieldLen, 28). Read64() doesn't
bounds-check m_pos against m_size, so a short ZIP64 extra field returns
uninitialised bytes from the header's 64-byte stack-allocated m_data and
they end up in the entry's m_Size / m_CompressedSize / m_Offset. Reject the
entry when fieldLen is below the requested 64-bit total.

Closes #26507.
  • 64b3221c
    by Vadim Zeitlin at 2026-05-25T14:46:55+02:00 
    Merge branch 'Review' of github.com:Blake-Madden/wxWidgets

Miscellaneous fixes mostly related to buffer sizes in wxMSW code.

See #26508.

10 changed files:

Changes:

  • src/common/imagiff.cpp
    ... ... @@ -468,7 +468,11 @@ int wxIFFDecoder::ReadIFF()
    468 468 
             const byte *bodyptr = dataptr + 8;          // -> BODY data
    469 469
    470 470 
             if (truncated) {
    471 
    -        chunkLen = dataend - dataptr;
    471 
    +        // Clamp the declared chunk length to the number of bytes actually
    472 
    +        // present after the BODY chunk header; otherwise the subsequent
    473 
    +        // decompression/decode loops would read up to 8 bytes (the size of
    474 
    +        // the chunk header) past the end of databuf.
    475 
    +        chunkLen = dataend - bodyptr;
    472 476 
             }
    473 477
    474 478 
             //

  • src/common/imagtga.cpp
    ... ... @@ -294,6 +294,18 @@ int ReadTGA(wxImage* image, wxInputStream& stream)
    294 294 
         // Load a palette if we have one.
    295 295 
         if (colorType == wxTGA_MAPPED)
    296 296 
         {
    297 
    +        // The palette buffer below is sized for paletteLength entries
    298 
    +        // indexed 0..paletteLength-1, and the wxPalette built from it
    299 
    +        // later (see SetPalette() below) is constructed starting at
    300 
    +        // index 0 as well. The loop writes entries at indices
    301 
    +        // [paletteStart, paletteStart+paletteLength), so any non-zero
    302 
    +        // paletteStart would cause Palette_SetRGB()/Palette_SetRGBA()
    303 
    +        // to write past the end of the buffer.
    304 
    +        if (paletteStart != 0)
    305 
    +        {
    306 
    +            return wxTGA_INVFORMAT;
    307 
    +        }
    308 
    +
    297 309 
             {
    298 310 
                 int palEntrySize = (palettebpp == 15 || palettebpp == 24) ? 3 : 4;
    299 311 
                 wxScopedArray<unsigned char> paletteTmp(paletteLength*palEntrySize);

  • src/common/zipstrm.cpp
    ... ... @@ -1026,7 +1026,28 @@ bool wxZipEntry::LoadExtraInfo(const char* extraData, wxUint16 extraLen, bool lo
    1026 1026 
                 }
    1027 1027
    1028 1028 
                 // Data block for extra field with Header ID = 1 (ZIP64)
    1029 
    -            // can have length up to 28 bytes.
    1029 
    +            // can have length up to 28 bytes. A ZIP64 extra field is only
    1030 
    +            // required to contain a 64-bit value for each of the size,
    1031 
    +            // compressed size and offset fields whose 32-bit value in the
    1032 
    +            // surrounding header was set to the ZIP64 sentinel 0xffffffff.
    1033 
    +            // Reject the entry if fieldLen is too small to cover those
    1034 
    +            // values, otherwise wxZipHeader::Read64() would walk past the
    1035 
    +            // end of its initialised data and yield uninitialised bytes
    1036 
    +            // from the stack-allocated header buffer.
    1037 
    +            size_t z64Required = 0;
    1038 
    +            if ( m_Size == 0xffffffff )
    1039 
    +                z64Required += 8;
    1040 
    +            if ( m_CompressedSize == 0xffffffff )
    1041 
    +                z64Required += 8;
    1042 
    +            if ( !localInfo && m_Offset == 0xffffffff )
    1043 
    +                z64Required += 8;
    1044 
    +            if ( fieldLen < z64Required )
    1045 
    +            {
    1046 
    +                wxLogWarning(_("Ignoring malformed extra data record, "
    1047 
    +                               "ZIP file may be corrupted"));
    1048 
    +                return false;
    1049 
    +            }
    1050 
    +
    1030 1051 
                 wxZipHeader ds(extraData+4, wxMin(fieldLen, 28));
    1031 1052 
                 // A file may contain larger size, compressed size or offset
    1032 1053 
                 // in a zip64 extra data block. Use the 64 bit values if available

  • src/msw/darkmode.cpp
    ... ... @@ -682,7 +682,7 @@ HandleMenuMessage(WXLRESULT* result,
    682 682 
                     WinStruct<MENUITEMINFO> mii;
    683 683 
                     mii.fMask = MIIM_STRING;
    684 684 
                     mii.dwTypeData = buf;
    685 
    -                mii.cch = sizeof(buf) - 1;
    685 
    +                mii.cch = WXSIZEOF(buf);
    686 686
    687 687 
                     // Note that we need to use the iPosition field of the
    688 688 
                     // undocumented struct here because DRAWITEMSTRUCT::itemID is

  • src/msw/ole/uuid.cpp
    ... ... @@ -18,6 +18,7 @@
    18 18
    19 19 
     #ifndef WX_PRECOMP
    20 20 
         #include "wx/msw/wrapwin.h"
    21 
    +    #include "wx/wxcrtvararg.h"
    21 22 
     #endif
    22 23
    23 24 
     #include  <rpc.h>                       // UUID related functions
    ... ... @@ -139,7 +140,7 @@ void Uuid::UuidToCForm()
    139 140 
       if ( m_pszCForm == nullptr )
    140 141 
         m_pszCForm = new wxChar[UUID_CSTRLEN];
    141 142
    142 
    -  wsprintf(m_pszCForm, wxT("0x%8.8X,0x%4.4X,0x%4.4X,0x%2.2X,0x2.2%X,0x2.2%X,0x2.2%X,0x2.2%X,0x2.2%X,0x2.2%X,0x2.2%X"),
    143 
    +  wxSnprintf(m_pszCForm, UUID_CSTRLEN, wxT("0x%8.8X,0x%4.4X,0x%4.4X,0x%2.2X,0x%2.2X,0x%2.2X,0x%2.2X,0x%2.2X,0x%2.2X,0x%2.2X,0x%2.2X"),
    143 144 
                m_uuid.Data1, m_uuid.Data2, m_uuid.Data3,
    144 145 
                m_uuid.Data4[0], m_uuid.Data4[1], m_uuid.Data4[2], m_uuid.Data4[3],
    145 146 
                m_uuid.Data4[4], m_uuid.Data4[5], m_uuid.Data4[6], m_uuid.Data4[7]);

  • src/msw/textctrl.cpp
    ... ... @@ -3611,10 +3611,10 @@ bool wxTextCtrl::GetStyle(long position, wxTextAttr& style)
    3611 3611
    3612 3612 
         LOGFONT lf;
    3613 3613 
         lf.lfWidth = 0;
    3614 
    -    lf.lfCharSet = ANSI_CHARSET; // FIXME: how to get correct charset?
    3614 
    +    lf.lfCharSet = (cf.dwMask & CFM_CHARSET) ? cf.bCharSet : DEFAULT_CHARSET;
    3615 3615 
         lf.lfClipPrecision = 0;
    3616 3616 
         lf.lfEscapement = 0;
    3617 
    -    wxStrcpy(lf.lfFaceName, cf.szFaceName);
    3617 
    +    wxStrlcpy(lf.lfFaceName, cf.szFaceName, WXSIZEOF(lf.lfFaceName));
    3618 3618
    3619 3619 
         //NOTE:  we _MUST_ set each of these values to _something_ since we
    3620 3620 
         //do not call wxZeroMemory on the LOGFONT lf

  • src/msw/webview_ie.cpp
    ... ... @@ -1783,9 +1783,9 @@ HRESULT STDMETHODCALLTYPE VirtualProtocol::ParseUrl(
    1783 1783 
                 case wxPARSE_SECURITY_URL:
    1784 1784 
                 case wxPARSE_SECURITY_DOMAIN:
    1785 1785 
                 {
    1786 
    -                if ( cchResult < secLen )
    1786 
    +                if ( cchResult <= secLen )
    1787 1787 
                         return S_FALSE;
    1788 
    -                wcscpy(pwzResult, m_handler->GetSecurityURL().wc_str());
    1788 
    +                wxStrlcpy(pwzResult, m_handler->GetSecurityURL().wc_str(), cchResult);
    1789 1789 
                     *pcchResult = secLen;
    1790 1790 
                     return S_OK;
    1791 1791 
                 }

  • src/unix/sound.cpp
    ... ... @@ -672,6 +672,11 @@ bool wxSound::LoadWAV(const void* data_, size_t length, bool copyData)
    672 672 
             data_offset += (list_chunk_length + 8u);
    673 673 
         }
    674 674
    675 
    +    // After skipping any LIST chunk we must still have room for the
    676 
    +    // 8-byte "data" chunk header that follows.
    677 
    +    if (length - data_offset < 8u)
    678 
    +        return false;
    679 
    +
    675 680 
         if (memcmp(&data[data_offset], "data", 4) != 0)
    676 681 
             return false;
    677 682

  • tests/archive/ziptest.cpp
    ... ... @@ -15,6 +15,7 @@
    15 15 
     #if wxUSE_STREAMS && wxUSE_ZIPSTREAM
    16 16
    17 17 
     #include "archivetest.h"
    18 
    +#include "wx/mstream.h"
    18 19 
     #include "wx/zipstrm.h"
    19 20
    20 21 
     #include <memory>
    ... ... @@ -250,4 +251,51 @@ CppUnit::Test *ziptest::makeTest(
    250 251 
     CPPUNIT_TEST_SUITE_REGISTRATION(ziptest);
    251 252 
     CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(ziptest, "archive/zip");
    252 253
    254 
    +TEST_CASE("Zip::BadZip64ExtraField", "[zip][error]")
    255 
    +{
    256 
    +    // wxZipEntry::LoadExtraInfo() used to handle a ZIP64 extra field (Header
    257 
    +    // ID = 1) that was too short to provide the 64-bit values the surrounding
    258 
    +    // header had asked for by calling wxZipHeader::Read64() anyway. Read64()
    259 
    +    // does not bounds-check against m_size, so it returned uninitialised
    260 
    +    // bytes from the wxZipHeader stack object's 64-byte m_data array, which
    261 
    +    // then ended up in m_Size, m_CompressedSize or m_Offset and was exposed
    262 
    +    // to callers through GetSize() / GetCompressedSize() / GetOffset().
    263 
    +    //
    264 
    +    // The central directory entry below sets compressed size and uncompressed
    265 
    +    // size to the ZIP64 sentinel 0xffffffff but pairs them with a zero-length
    266 
    +    // ZIP64 extra field. After the fix the size fields are left at the
    267 
    +    // sentinel value rather than overwritten with whatever Read64() happened
    268 
    +    // to scrape off the stack.
    269 
    +    static const unsigned char data[] = {
    270 
    +        // Local file header for entry "a"
    271 
    +        'P','K',0x03,0x04, 0x14,0x00, 0x00,0x00, 0x00,0x00,
    272 
    +        0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,
    273 
    +        0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,
    274 
    +        0x01,0x00, 0x00,0x00, 'a',
    275 
    +        // Central directory entry
    276 
    +        'P','K',0x01,0x02, 0x14,0x00, 0x14,0x00, 0x00,0x00, 0x00,0x00,
    277 
    +        0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,
    278 
    +        0xff,0xff,0xff,0xff, 0xff,0xff,0xff,0xff,
    279 
    +        0x01,0x00, 0x04,0x00, 0x00,0x00, 0x00,0x00, 0x00,0x00,
    280 
    +        0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00,
    281 
    +        'a',
    282 
    +        // Extra field: ID=1 (ZIP64), fieldLen=0
    283 
    +        0x01,0x00, 0x00,0x00,
    284 
    +        // End of central directory record
    285 
    +        'P','K',0x05,0x06, 0x00,0x00, 0x00,0x00, 0x01,0x00, 0x01,0x00,
    286 
    +        0x33,0x00,0x00,0x00, 0x1f,0x00,0x00,0x00,
    287 
    +        0x00,0x00,
    288 
    +    };
    289 
    +
    290 
    +    wxMemoryInputStream mis(data, sizeof(data));
    291 
    +    wxZipInputStream zip(mis);
    292 
    +    std::unique_ptr<wxZipEntry> entry(zip.GetNextEntry());
    293 
    +    REQUIRE( entry );
    294 
    +    // Without the validation in LoadExtraInfo() the eight bytes returned by
    295 
    +    // Read64() are uninitialised: assert the ZIP64 sentinel is left alone so
    296 
    +    // that the malformed extra field cannot poison the entry's size fields.
    297 
    +    CHECK( entry->GetSize() == wxFileOffset(0xffffffff) );
    298 
    +    CHECK( entry->GetCompressedSize() == wxFileOffset(0xffffffff) );
    299 
    +}
    300 
    +
    253 301 
     #endif // wxUSE_STREAMS && wxUSE_ZIPSTREAM

  • tests/image/image.cpp
    ... ... @@ -1187,6 +1187,48 @@ TEST_CASE_METHOD(ImageHandlersInit, "wxImage::ReadCorruptedTGA", "[image]")
    1187 1187 
         */
    1188 1188 
         corruptTGA[18] = 0x7f;
    1189 1189 
         REQUIRE( !tgaImage.LoadFile(memIn) );
    1190 
    +
    1191 
    +    /*
    1192 
    +    A colour-mapped TGA with a non-zero colour-map origin.
    1193 
    +    Older code allocated the palette using paletteLength only, but
    1194 
    +    indexed it using paletteStart + i, leading to OOB writes.
    1195 
    +    */
    1196 
    +    static const unsigned char badPaletteTGA[] =
    1197 
    +    {
    1198 
    +        0,          // ID length
    1199 
    +        1,          // Color map type
    1200 
    +        1,          // Image type = color mapped
    1201 
    +
    1202 
    +        1, 0,       // Color map origin (paletteStart = 1)
    1203 
    +        1, 0,       // Color map length = 1 entry
    1204 
    +        24,         // Color map entry size
    1205 
    +
    1206 
    +        0, 0,       // X-origin
    1207 
    +        0, 0,       // Y-origin
    1208 
    +
    1209 
    +        1, 0,       // Width = 1
    1210 
    +        1, 0,       // Height = 1
    1211 
    +
    1212 
    +        8,          // Bits per pixel
    1213 
    +        0,          // Image descriptor
    1214 
    +
    1215 
    +        // One palette entry (BGR)
    1216 
    +        0xff, 0x00, 0x00,
    1217 
    +
    1218 
    +        // One pixel index
    1219 
    +        0x00
    1220 
    +    };
    1221 
    +
    1222 
    +    wxMemoryInputStream badPaletteStream(
    1223 
    +        badPaletteTGA,
    1224 
    +        WXSIZEOF(badPaletteTGA)
    1225 
    +    );
    1226 
    +
    1227 
    +    REQUIRE( badPaletteStream.IsOk() );
    1228 
    +
    1229 
    +    REQUIRE(
    1230 
    +        !tgaImage.LoadFile(badPaletteStream, wxBITMAP_TYPE_TGA)
    1231 
    +    );
    1190 1232 
     }
    1191 1233
    1192 1234 
     #if wxUSE_GIF
    ... ... @@ -1471,6 +1513,46 @@ TEST_CASE_METHOD(ImageHandlersInit, "wxImage::BadIFFBmhdOverflow",
    1471 1513 
         REQUIRE( !img.LoadFile(mis, wxBITMAP_TYPE_IFF) );
    1472 1514 
     }
    1473 1515
    1516 
    +TEST_CASE_METHOD(ImageHandlersInit, "wxImage::BadIFFBodyTruncated",
    1517 
    +                 "[image][iff][error]")
    1518 
    +{
    1519 
    +    // A BODY chunk that declares more data than the file actually contains:
    1520 
    +    // dataptr + 8 + chunkLen > dataend, so the truncated branch in
    1521 
    +    // wxIFFDecoder::ReadIFF() runs and used to set chunkLen = dataend - dataptr,
    1522 
    +    // which is 8 too large (the chunk header is 8 bytes). The non-RLE BODY
    1523 
    +    // decode loop then reads up to 8 bytes past the heap-allocated input
    1524 
    +    // buffer. width = 16 with 1 bitplane gives lineskip = 2, and the second
    1525 
    +    // half of the scanline (j = 8..15) reads bodyptr[1], one byte past dataend.
    1526 
    +    // transparentColor = 0x4000 ensures ConvertToImage() also rejects the
    1527 
    +    // resulting image so the test fails cleanly with the fix applied.
    1528 
    +    wxImage::AddHandler(new wxIFFHandler);
    1529 
    +
    1530 
    +    static const unsigned char data[] =
    1531 
    +    {
    1532 
    +        0x46,0x4f,0x52,0x4d,            // "FORM"
    1533 
    +        0x00,0x00,0x00,0x29,            // FORM length (not validated)
    1534 
    +        0x49,0x4c,0x42,0x4d,            // "ILBM"
    1535 
    +        0x42,0x4d,0x48,0x44,            // "BMHD"
    1536 
    +        0x00,0x00,0x00,0x14,            // BMHD chunk length = 20
    1537 
    +        0x00,0x10,                      // width  = 16
    1538 
    +        0x00,0x01,                      // height = 1
    1539 
    +        0x00,0x00,0x00,0x00,            // x, y
    1540 
    +        0x01,                           // nPlanes
    1541 
    +        0x00,                           // masking
    1542 
    +        0x00,                           // compression (uncompressed)
    1543 
    +        0x00,                           // pad
    1544 
    +        0x40,0x00,                      // transparentColor = 0x4000
    1545 
    +        0x00,0x00,                      // x/y aspect
    1546 
    +        0x00,0x00,0x00,0x00,            // page width / height
    1547 
    +        0x42,0x4f,0x44,0x59,            // "BODY"
    1548 
    +        0x00,0x00,0x00,0x64,            // BODY chunk length = 100 (lie)
    1549 
    +        0xff,                           // 1 byte of body data
    1550 
    +    };
    1551 
    +    wxMemoryInputStream mis(data, WXSIZEOF(data));
    1552 
    +    wxImage img;
    1553 
    +    REQUIRE( !img.LoadFile(mis, wxBITMAP_TYPE_IFF) );
    1554 
    +}
    1555 
    +
    1474 1556 
     #endif // wxUSE_IFF
    1475 1557
    1476 1558 
     TEST_CASE_METHOD(ImageHandlersInit, "wxImage::DibPadding", "[image]")

Reply all
Reply to author
Forward
0 new messages