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.
This commit is contained in:
dxbjavid
2026-05-23 20:55:04 +02:00
committed by Vadim Zeitlin
parent 294a364b48
commit b2d7c29f29
2 changed files with 54 additions and 0 deletions
+12
View File
@@ -294,6 +294,18 @@ int ReadTGA(wxImage* image, wxInputStream& stream)
// Load a palette if we have one.
if (colorType == wxTGA_MAPPED)
{
// The palette buffer below is sized for paletteLength entries
// indexed 0..paletteLength-1, and the wxPalette built from it
// later (see SetPalette() below) is constructed starting at
// index 0 as well. The loop writes entries at indices
// [paletteStart, paletteStart+paletteLength), so any non-zero
// paletteStart would cause Palette_SetRGB()/Palette_SetRGBA()
// to write past the end of the buffer.
if (paletteStart != 0)
{
return wxTGA_INVFORMAT;
}
{
int palEntrySize = (palettebpp == 15 || palettebpp == 24) ? 3 : 4;
wxScopedArray<unsigned char> paletteTmp(paletteLength*palEntrySize);
+42
View File
@@ -1187,6 +1187,48 @@ TEST_CASE_METHOD(ImageHandlersInit, "wxImage::ReadCorruptedTGA", "[image]")
*/
corruptTGA[18] = 0x7f;
REQUIRE( !tgaImage.LoadFile(memIn) );
/*
A colour-mapped TGA with a non-zero colour-map origin.
Older code allocated the palette using paletteLength only, but
indexed it using paletteStart + i, leading to OOB writes.
*/
static const unsigned char badPaletteTGA[] =
{
0, // ID length
1, // Color map type
1, // Image type = color mapped
1, 0, // Color map origin (paletteStart = 1)
1, 0, // Color map length = 1 entry
24, // Color map entry size
0, 0, // X-origin
0, 0, // Y-origin
1, 0, // Width = 1
1, 0, // Height = 1
8, // Bits per pixel
0, // Image descriptor
// One palette entry (BGR)
0xff, 0x00, 0x00,
// One pixel index
0x00
};
wxMemoryInputStream badPaletteStream(
badPaletteTGA,
WXSIZEOF(badPaletteTGA)
);
REQUIRE( badPaletteStream.IsOk() );
REQUIRE(
!tgaImage.LoadFile(badPaletteStream, wxBITMAP_TYPE_TGA)
);
}
#if wxUSE_GIF