It's not clear whether we handle them correctly, and so it might be
better to refuse input with duplicated groups entirely, but for now at
least avoid asserting when encountering them.
Reading from a truncated stream returned values built from partially
or completely uninitialised buffers, e.g. ReadString() decoded the
unread tail of its temporary buffer and the fixed size readers returned
whatever happened to be on the stack. Check that all the requested bytes
were actually read and, if not, mark the input stream as being in error
and return an empty/zero value instead of silently wrong data.
Closes#26600.
This function allocates the palette r/g/b arrays from the colour count
given in the XPM header but fills them by walking the colour map, so a
malformed file that reuses the same key on more than one colour line
collapses those lines into a single map entry and leaves the tail of the
arrays uninitialised. wxPalette() then copies that uninitialised memory
into the palette later returned by GetPalette(), and in debug builds the
wxASSERT(i == colors_cnt) at the end fires.
Use the actual number of distinct colours in the map as the size for
the arrays and the palette to fix this.
Also add a test loading an XPM triggering this issue.
Closes#26595.
Add a new OSS-Fuzz fuzzer targeting tar operation, mirroring the
existing zip fuzzer.
Closes#26579.
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
LoadCachedBook reads each index entry's parent back-reference straight
from the .cached file and uses it as an index with no check, so a
crafted cached file (which can sit inside a .htb help archive opened
through AddBook) may result in creating a pointer to data outside the
m_index array; that pointer is later dereferenced when the index is
sorted at the end of AddBookParam.
The change rejects the file when parentShift is negative or larger than
the number of index entries loaded so far, matching the existing version
and flags checks just above.
Also add a small test under tests/html that feeds such a cached stream
and confirms it is now refused rather than reading out of bounds.
Closes#26577.
wxRegExImpl::Replace() scans replacement.c_str() and does *++p after a
backslash. When the replacement ends in a lone backslash, that reads the
terminating NUL, the else branch appends it, and the loop's p++ then
steps one byte past the NUL so the *p condition reads out of bounds.
Keep a trailing backslash verbatim and stop before the increment.
Add a test checking that this doesn't result in ASAN errors any more.
Closes#26541.
The image palette can have more than the 256 entries a GIF supports, for
example when the image was loaded from an XPM that declares a larger
colour count. wxGIFHandler_GetPalette(), called when saving a GIF,
copied every entry into the caller's fixed wxRGB pal[256] buffer,
overflowing it.
Such an image can't be represented as a GIF, so reject the save with an
error message instead of overflowing the buffer or writing out a silently
truncated palette.
Closes#26532.
Don't write NUL byte to "buf[size - 1]" when size is 0.
This bug was present in both implementations of wxVsnprintf(), so fix it
in both places too.
Closes#26522.
A command ending in a bare '%' made the loop advance in this function
advance past the end of string.
Fix this by handling only non-trailing '%' specifically
Closes#26531.
recSize in wxTarInputStream::ReadExtendedHeader() comes from the
record's decimal byte-count field, so a pax header declaring a length
close to SIZE_MAX makes recPos + recSize wrap below len and pass the
check. The following pRec[recSize - 1] then reads far past the buffer,
which ASAN flags as a heap-buffer-overflow.
Comparing recSize against len - recPos avoids the wrap.
Closes#26530.
We need to use memmove() and not strncpy() in wxXPMDecoder code as
nothing guarantees that the source and destination regions don't overlap
and they did overlap, in fact, for the test case added as part of
46f928d057 (Fix buffer overflow on invalid width in wxXPMDecoder,
2026-05-27).
Also make the test itself more explicit by hard-coding the test XPM
instead of constructing it dynamically.
See #26519.
Fix integer overflow in wxXPMDecoder::ReadData() when computing
width*chars_per_pixel: a header with a (very) large width could result
in wrapping around and result in reasonable but yet invalid value, which
could let the key-reading loop run off the end of the buffer.
Compute the product in 64-bits to avoid the overflow.
Closes#26519.
Avoid integer overflow in message catalog StringAtOfs() bounds check:
StringAtOfs() read ofsString and nLen from the .mo file and checked
ofsString + nLen against the data length, but both are 32-bit so the
sum wraps and a translated entry declaring nLen 0xffffffff passes the
check; FillHash() then scans past the end of the catalog. Compute the
offset as a 64-bit value so the addition can't overflow and result in an
invalid index into the buffer.
Add a test feeding such a catalog through wxMsgCatalog::CreateFromData().
Closes#26513.
The replicate-run branch reads the packet's data byte after only
checking that one source byte remains, but it consumes two (the control
byte plus the data byte). A BODY ending in a lone replicate control byte
then reads one byte past the input buffer. Require two bytes left,
matching the literal-run check above.
Closes#26518.
In wxMBConvUTF7::ToWChar() the value of the byte after '+' was cast to
"unsigned", which meant that on the platforms with signed bytes values
greater than 0x80 were sign-extended to a ~4GiB index which was (way)
out of bounds for a 256-entry table.
Fix the code by casting to "unsigned char", like the cc lookup just
above already does.
Closes#26517.
The SEQ chunk of an ANI file gives a 32-bit image index per animation
step. These values were stored into wxANIFrameInfo::m_imageIndex
verbatim, without any check against the number of icon chunks actually
loaded into m_images. wxANIDecoder::ConvertToImage() and
GetTransparentColour() then used the value as an index into m_images
directly, so a malformed ANI file could trigger an out-of-bounds vector
access when the file is displayed.
Reject the file in Load() if any of the indices is negative or points
past the end of m_images, and also reject files that produced no icon
chunks at all so the subsequent m_images[0] reference is safe.
Closes#26492.
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.
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.
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.
The minimum code size byte that follows the local colour table in
wxGIFDecoder::LoadGIF() is only checked for <= 0. dgif() sizes
ab_prefix/ab_tail with allocSize = 4096 + 1, so a value of 12
starts ab_free at 4098 and the first alphabet update at
gifdecod.cpp:457 writes one entry past the end of both arrays. The
existing wxASSERT(ab_free < allocSize) already flagged this in debug
builds. The new wxImage::BadGIFLZWMinCodeSize test feeds a 37-byte
2x1 GIF with code size 12 to LoadFile and asserts it is rejected.
Closes#26501.
The quote-stripping loop in wxXPMDecoder::ReadFile() sets p = q + 1
after strncpy(). If the closing " was missing, q stopped at the
buffer terminator, so p ended up one past it and the outer for-loop's
p++ then dereferenced two bytes off the end of the wxCharBuffer.
Mirror the already-existing /*-comment treatment and break out of the
loop when *q == '\0'.
Closes#26499.
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.
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.
Following review on the previous commit, switch from silently ignoring
an out-of-bounds BMHD transparent colour index to rejecting the file
outright in wxIFFDecoder::ConvertToImage. The earlier "ignore" approach
let the load succeed, which made the BadIFF test pass only as a side
effect of corruption from the unrelated overrun and so failed
intermittently in some wxMSW builds.
Apply the bounds check using the form suggested in the PR review.
The transparent colour index from the IFF BMHD chunk is a 16-bit
value stored unclamped in m_image->transparent. Using it as a
palette index without checking it against the CMAP-derived colour
count writes 3 bytes at an attacker-controlled offset past the
palette buffer.
Validate the index against the actual palette size before applying
the magenta mask.
Don't mirror coordinates passed to GDI functions in RTL layout as they
already mirror them internally, resulting in incorrect coordinates being
used. As a consequence, wxDC::LogicalToDevice() now always returns
unmirrored coordinates, even in RTL layout, which is consistent with
wxGTK3 and wxQt.
Also fix scaling even in LTR layout by using ::ScaleWindowExtEx().
See #26398.
Vadim Zeitlin
Arthur ChanandVadim Zeitlin
dxbjavidandVadim Zeitlin
Paul Cornett
Blake-MaddenandVadim Zeitlin
Maarten Bent
ryancogandVadim Zeitlin
jmestwa-coderandVadim Zeitlin
MarkLee131