The primaryNdx and approxNdx fields in the shape wire description are
attacker-controlled CARD8 values from the client request. They are used
to index into the shape->outlines[] array, but were only checked against
XkbNoShape (0xff) and never validated against the actual number of
outlines (shapeWire->nOutlines).
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Off-by-one in rowUnder validation: the bounds check uses '>' instead
of '>=' when comparing rWire->rowUnder against section->num_rows.
Since num_rows is a count and valid indices are 0 to num_rows-1,
rowUnder == num_rows passes the check but is one past the valid range.
XkbAddGeomOverlayRow() uses this as an array index, causing an
out-of-bounds read on section->rows[].
And throw in two alloc checks while we're at it.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The bounds checks for baseColorNdx and labelColorNdx in _CheckSetGeom()
use '>' instead of '>=' when comparing against req->nColors. Since
nColors is a count and valid indices are 0 to nColors-1, an index equal
to nColors is one past the end of the array.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
value points to the location of the client PID, assign it first before
we swap it. For consistency move the memcpy up too so the copy commands
are all in the same location.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
CheckKeyActions() validates the per-key action count bytes individually
but does not verify that the computed total action data region falls
within the request buffer before advancing the wire pointer past it.
After the loop, the function calculates the final wire position as
wire + nActs * sizeof(XkbAnyAction), where nActs is the sum of per-key
action counts read from the request. The upstream length validation in
_XkbSetMapCheckLength() uses req->totalActs from the request header,
not the computed nActs. If a crafted request provides a totalActs value
that passes the length check but per-key action counts that sum to a
different nActs, the wire pointer could advance past the actual request
buffer.
The subsequent SetKeyActions() function uses memcpy to read from this
potentially out-of-bounds region, which could leak heap data or cause
a crash.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The for loop here always iterates size times but the client controls the
starting offset. When the starting pixel is non-zero (e.g., pixel=10 in
a size=256 colormap), the loop reads from pentFirst[10] through
pentFirst[265], reading 10 entries past the end of the array.
Fix this by wrapping around once we reach size, same as FindColor()
already does.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2215>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
AllocARGBCursor took ownership of the psrcbits/pmaskbits/argb arguments.
But if the initial calloc failed none of them were freed, without the
caller knowing about it. Depending on the code path, those arguments
would thus either leak or be double-freed.
Fix it by always freeing those on error and updating the callers
accordingly.
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2214>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
present_create_notifies() creates an array of notifies but never returns
them to the caller, despite them being passed individually to
present_add_window_notify(). The caller proceeds with a NULL notifies
array, eventually causing an OOB in present_vblank_notify() when
vblank->notifies is NULL.
Reported-by: Feng Ning, Innora Pte. Ltd.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2204>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need to spin for a zero-length change. The loop is already bounded to
255 iterations so this just keeps the room slightly cooler.
This is a slight behavior change in that subsequent hierarchy changes
will no longer be accepted (but already-applied changes remain). But a
client sending zero length hierarchy changes is buggy anyway, so meh.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
In siHostnameCheckAddr(), the digit validation range was 0x30-0x3A, but
0x3A is the colon character (':'). The ASCII range for digits 0-9 is
0x30-0x39.
Colons in hostnames violate RFC 2396 section 3.2.2 and we're not parsing
the host:port notation here.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
There is no OOB write, the loop a few lines below has the correct
i < numAxes check. But this does set last_valuator to an invalid
value which may have flow-on effects elsewhere later.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No real effect here since we check stuff->format early in
ProcRRChangeProviderProperty anyway. But this just makes it a bit more
obvious (and more consistent with other functions).
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The primary_ndx and approx_ndx fields from the XKM shape wire
description are used as indices into the shape->outlines[] array without
bounds checking against num_outlines.
Exploiting this (if it can be exploited) requires a malicious xkbcomp -
the path of which is built-in at compile time. There are lower-hanging
targets than trying to exploit through an XKM file.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2207>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Both FreeCounter() and miSyncDestroyFence() iterate over the trigger list
and invoke the CounterDestroyed callback on each trigger.
The CounterDestroyed callback (e.g. SyncAwaitTriggerFired) may call
FreeResource/FreeAwait, which frees the SyncAwaitUnion containing all
SyncAwait structs in the same Await group.
When multiple conditions in a single Await reference the same sync
object (counter or fence), the first callback frees all SyncAwait
structs while subsequent trigger list nodes still reference them. On the
next iteration, reading ptl->next or ptl->pTrigger dereferences freed
memory, leading to a use-after-free.
We need separate fixes for separate issues here to fix this in one go
- use our null-terminated list macro to make sure our next pointer stays
valid (the code accessed ptl->next after freeing it)
- update the list head before deleting the trigger, eventually this ends
up being NULL anyway but meanwhile the list head is a valid list
during CounterDestroyed
- check if we actually do have a trigger before dereferencing the
callback
- Set all triggers to NULL if they are shared so we don't dereference
potentially freed memory
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30159 (miSyncDestroyFence), ZDI-CAN-30163 (FreeCounter)
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
CheckKeyTypes computes nMaps = firstType + nTypes from client-controlled
request fields when XkbSetMapResizeTypes is set. This value is used to
index mapWidths[], a stack-allocated CARD8 array of XkbMaxLegalKeyCode + 1
(256) elements. No upper bound is enforced on nMaps.
An attacker can first send SetMap(firstType=0, nTypes=255, ResizeTypes) to
set the server's num_types to 255, then send SetMap(firstType=255,
nTypes=10, ResizeTypes). The firstType > num_types check passes because
255 > 255 is false (the check uses > rather than >=). nMaps is then
computed as 265, and the loop writes mapWidths[255..264], overflowing 9
bytes past the stack buffer into adjacent stack variables (symsPerKey[]).
Fix by rejecting requests where firstType + nTypes would exceed the
mapWidths buffer size (XkbMaxLegalKeyCode + 1).
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30161
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The request length validation in __glXDisp_ChangeDrawableAttributes and
__glXDispSwap_ChangeDrawableAttributes uses the wrong comparison direction.
The check tests whether the computed request size is LESS THAN
client->req_len, but should test whether it is GREATER THAN. With the
reversed operator, an undersized request (where numAttribs claims more
attribute pairs than the request actually contains) passes validation.
DoChangeDrawableAttributes then iterates numAttribs attribute pairs starting
from the end of the request header, reading past the actual request data
into adjacent memory. This is an out-of-bounds read that can also cause
an out-of-bounds write when a GLX_EVENT_MASK attribute key is found in the
overread data and its corresponding value is written to pGlxDraw->eventMask.
This patch effectively reverts commit 402b329c3a ("glx: Work around
wrong request lengths sent by mesa"). This was fixed in mesa commit
4324d6fdfbba1 in 2011 (mesa 7.11).
Fixes: 402b329c3a ("glx: Work around wrong request lengths sent by mesa")
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30165
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
CreateSaverWindow stores pPriv (the ScreenSaverScreenPrivatePtr) in a local
variable via the SetupScreen macro at function entry. When an existing saver
window is being replaced, the function sets pPriv->hasWindow = FALSE and
calls CheckScreenPrivate(). If at this point pPriv->attr is NULL (cleared
by a prior UnsetAttributes call), pPriv->events is NULL, and
pPriv->installedMap is None, then CheckScreenPrivate determines the screen
private is unused, frees it, and sets the screen private pointer to NULL.
The function then continues to dereference the now-freed pPriv on the very
next line (pPriv->attr), resulting in a use-after-free. On glibc 2.34+,
the tcache key at offset 8 within the freed block makes pPriv->attr appear
non-NULL, causing the function to continue operating on garbage data and
eventually crash.
The attack sequence is:
1. SetAttributes (creates pPriv with pPriv->attr set)
2. ForceScreenSaver(Active) (creates saver window, pPriv->hasWindow=TRUE)
3. UnsetAttributes (sets pPriv->attr = NULL)
4. ForceScreenSaver(Active) (re-enters CreateSaverWindow → UAF)
Fix by re-fetching pPriv from the screen private after CheckScreenPrivate
returns, so the subsequent NULL check correctly detects the freed state.
ScreenSaverFreeAttr has the same pattern, force pPriv to NULL there too
even though it has no real effect.
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30168
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This is the equivalent check to miSyncTriggerFence() from
commit f19ab94ba9 ("miext/sync: Fix use-after-free in miSyncTriggerFence()")
When a trigger fires via SyncAwaitTriggerFired, the resulting
FreeResource/FreeAwait call invokes SyncDeleteTriggerFromSyncObject for
every trigger in the same Await group. This unlinks and frees the
corresponding trigger list nodes - potentially including the node pnext
points to.
Fix by restarting iteration from the list head after a trigger fires, since
TriggerFired may have arbitrarily mutated the list. Triggers that have fired
are removed from the list by FreeAwait, so restarting cannot cause infinite
loops.
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30164
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
CheckKeyTypes validates incoming key type definitions from XkbSetMap
requests but does not enforce an upper bound on numLevels. A client can set
numLevels up to 255 on a non-canonical key type, which is stored in the
server's type table.
When ChangeKeyboardMapping later triggers XkbUpdateKeyTypesFromCore, the
function XkbKeyTypesForCoreSymbols computes groupsWidth from num_levels and
uses the XKB_OFFSET(g, l) = (g * groupsWidth) + l macro to index into
tsyms[], a stack-allocated buffer of XkbMaxSymsPerKey (252) entries. With
num_levels=255, groupsWidth=255, and indices reach up to 3*255+254 = 1019,
overflowing the 252-element stack buffer by 767 KeySym-sized entries.
Fix by rejecting numLevels values greater than XkbMaxShiftLevel (63) in
CheckKeyTypes, alongside the existing lower-bound check for numLevels < 1.
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30160
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
XLFDMAXFONTNAMELEN was 256 bytes, but libXfont2 defines MAXFONTNAMELEN
as 1024 and allows font names and alias targets up to that length in
fonts.alias files.
doListFontsAndAliases copies the resolved alias target into a
stack-allocated tmp_pattern[XLFDMAXFONTNAMELEN] and then into
c->current.pattern[XLFDMAXFONTNAMELEN] (defined in LFWIstateRec).
doListFontsWithInfo has the same pattern, copying the resolved name into
c->current.pattern[]. With the old 256-byte limit, a fonts.alias entry
with a target name between 257 and 1023 bytes would overflow both
buffers.
An attacker can exploit this by:
1. Creating a font directory with a fonts.alias containing an alias
whose target name exceeds 256 bytes
2. Using SetFontPath to add the malicious directory
3. Calling ListFonts with the alias name to trigger alias resolution
4. The oversized resolved name overflows the 256-byte stack buffer
Increase XLFDMAXFONTNAMELEN from 256 to 1024 to match libXfont2's
MAXFONTNAMELEN, ensuring the server can handle any name the font library
produces.
This vulnerability was discovered by:
Anonymous working with TrendAI Zero Day Initiative
ZDI-CAN-30136
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
_Concat() stored the realloc() result directly in its input pointer, so
an allocation failure could drop the only reference to the original
buffer when callers assigned the return value back to their destination
pointer.
Keep the old pointer until realloc() succeeds, avoiding the leak
reported by static analysis.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2221>
ospoll_create() initializes backend-specific state immediately after
allocating the ospoll structure. Check the allocation result for each
backend before dereferencing it and return NULL on failure.
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2217>
This works as intended — the (fake) front buffer needs to be added
only if the client didn't request it in the first place — even if the
client requests the same attachment multiple times. This ensures we
never try to access more than (count + 1) entries of the buffers array.
Fixes: ff6c7764c2 ("DRI2: Implement protocol for DRI2GetBuffersWithFormat")
Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
This was fixed in commit 51eb63b0ee but woefully badly. Instead of returning
XIAlreadyGrabbed via the Reply, it simply returned the value from the
request handler - causing the server to interpret it as BadRequest.
Fix it and do what we intended to do instead.
Fixes: 51eb63b0ee ("Xi: disallow passive grabs with a detail > 255")
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2186>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
When the target device is disabled, ProcXIGrabDevice returns
AlreadyGrabbed directly as the request handler return value.
AlreadyGrabbed (1) is a grab status code, not an X error code. The
server dispatch loop interprets any non-zero return value as an X
protocol error, so the client receives BadRequest (error code 1)
instead of a proper XIGrabDevice reply with status=AlreadyGrabbed.
And use XIAlreadyGrabbed since this is an XI2 request. It's the same
value anyway.
This is the same class of bug that was fixed in ProcXIPassiveGrabDevice
by commit 'Xi: Fix XIPassiveGrab handling of keycodes > 255'
Fix by jumping to the reply path with status=AlreadyGrabbed instead of
returning the status code directly.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2186>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
XLibre 10-nvidia.conf is extended w.r.t. XOrg version and can be overwritten by
NVidia installers or packages, hence it is safer to split it into 10-nvidia.conf
(a common part) and 10-nvidia-modules.conf (an extra part).
This patch also provides /etc/X11/xorg.conf.d/10-nvidia.conf.debian and
/etc/X11/xorg.conf.d/10-nvidia-modules.conf.debian files that, when .debian
suffix is removed, shadow the respective files in /usr/share/X11/xorg.conf.d
and set Debian specific search directories for drivers and modules.
Signed-off-by: Oleh Nykyforchyn <oleh.nyk@gmail.com>
If we fail to set up the default rules our keymap is likely going to end
up messed up, which means the client/user can't work correctly anyway.
And if we're that low on memory that we can't allocate these rules,
we're about to fall over anyway so why bother.
Co-Authored-by: Claude Code <noreply@anthropic.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2184>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Moved the cursor glyph cropping into a separate function that handles reflections and rotations.
Also, x/y are indexes into the buffer, so they should be clamped to width/height len - 1 for correctness.
Signed-off-by: John Studnicka <contact@zentec.dev>
Use the XNF version for this and simply bail out if it fails. Clearly
this hasn't been a problem in over 20 years and I can't be bothered
finding the perfect cleanup path.
Co-Authored-by: Claude Code <noreply@anthropic.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2184>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
CHK_MASK_LEGAL expands to 'return BadValue' when the check fails and
doesn't clean up the already allocated names.keycodes, names.types, etc.
Move the check up so we don't need any cleanup code.
Co-Authored-by: Claude Code <noreply@anthropic.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2184>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
On allocation error, also have to remove devices and possibly
clear out some other pointers.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Finally, after 33 years something deals with the allocation failure. Put
on a party hat!
Co-Authored-by: Claude Code <noreply@anthropic.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2184>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
During extension init this makes sense, failing to assign a name to a
new device is more controversial but none of the paths handle
this situation correctly right now so we're just as likely to introduce
an exploit if the name remains NULL.
Co-Authored-by: Claude Code <noreply@anthropic.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2184>
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
