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
(cherry picked from commit 867b59b33b)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2229>
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
(cherry picked from commit 543e108516)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2229>
Reported in #1817:
xwayland-24.1.6/redhat-linux-build/../xkb/xkbInit.c:527:5:
warning[-Wanalyzer-malloc-leak]: leak of ‘rmlvo_dflts.layout’
xwayland-24.1.6/redhat-linux-build/../xkb/xkbInit.c:527:5:
warning[-Wanalyzer-malloc-leak]: leak of ‘rmlvo_dflts.model’
xwayland-24.1.6/redhat-linux-build/../xkb/xkbInit.c:527:5:
warning[-Wanalyzer-malloc-leak]: leak of ‘rmlvo_dflts.options’
xwayland-24.1.6/redhat-linux-build/../xkb/xkbInit.c:527:5:
warning[-Wanalyzer-malloc-leak]: leak of ‘rmlvo_dflts.rules’
xwayland-24.1.6/redhat-linux-build/../xkb/xkbInit.c:527:5:
warning[-Wanalyzer-malloc-leak]: leak of ‘rmlvo_dflts.variant’
Fixes: 56a5955c8 ("xkb: strdup the values returned by XkbGetRulesDflts")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2169>
(cherry picked from commit 97cf051dfc)
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
(cherry picked from commit a439a7340a)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2224>
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
(cherry picked from commit ed19312c4b)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2224>
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
(cherry picked from commit 6b6e8020b9)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2224>
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
(cherry picked from commit 86a321ad98)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2224>
len calculation and strncpy limit were off by one when prefixing
"vmod_" to the virtual modifier name. This could write the final
NULL one byte past the allocated buffer from tbGetBuffer().
Use proper allocation len for prefix to avoid writing out-of-bounds.
Found by Linux Verification Center (linuxtesting.org) with SVACE
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
(cherry picked from commit 5dfb435c1d)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2224>
In XkbAddGeomDoodad(), when adding a doodad to a specific section
(section != NULL), there is a comparison between section->num_doodads
and geom->sz_doodads instead of the section's own section->sz_doodads.
The else branch (global geometry doodads) was already correct.
Compare section->num_doodads against section->sz_doodads to prevent
a potential out-of-bounds.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
(cherry picked from commit dd8b8cf49d)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2224>
Similar to the recent fixes, add more _XkbCheckRequestBounds() to the
functions that loop over the request data, i.e.:
* CheckKeySyms()
* CheckKeyActions()
* CheckKeyBehaviors()
* CheckVirtualMods()
* CheckKeyExplicit()
* CheckVirtualModMap()
* _XkbSetMapChecks()
All these are static functions so we can add the client to the parameters
without breaking any API.
See also:
CVE-2026-34003, ZDI-CAN-28736, CVE-2026-34002, ZDI-CAN-28737
v2: Check for "nSyms != 0" in CheckKeySyms() to avoid false positives.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit d38c563fab)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2178>
The function CheckKeyTypes() will loop over the client's request but
won't perform any additional bound checking to ensure that the data
read remains within the request bounds.
As a result, a specifically crafted request may cause CheckKeyTypes() to
read past the request data, as reported by valgrind:
== Invalid read of size 2
== at 0x5A3D1D: CheckKeyTypes (xkb.c:1694)
== by 0x5A6A9C: _XkbSetMapChecks (xkb.c:2515)
== by 0x5A759E: ProcXkbSetMap (xkb.c:2736)
== by 0x5BF832: SProcXkbSetMap (xkbSwap.c:245)
== by 0x5C05ED: SProcXkbDispatch (xkbSwap.c:501)
== by 0x4A20DF: Dispatch (dispatch.c:551)
== by 0x4B03B4: dix_main (main.c:277)
== by 0x428941: main (stubmain.c:34)
== Address is 30 bytes after a block of size 28,672 in arena "client"
==
== Invalid read of size 2
== at 0x5A3AB6: CheckKeyTypes (xkb.c:1669)
== by 0x5A6A9C: _XkbSetMapChecks (xkb.c:2515)
== by 0x5A759E: ProcXkbSetMap (xkb.c:2736)
== by 0x5BF832: SProcXkbSetMap (xkbSwap.c:245)
== by 0x5C05ED: SProcXkbDispatch (xkbSwap.c:501)
== by 0x4A20DF: Dispatch (dispatch.c:551)
== by 0x4B03B4: dix_main (main.c:277)
== by 0x428941: main (stubmain.c:34)
== Address is 2 bytes after a block of size 28,672 alloc'd
== at 0x4848897: realloc (vg_replace_malloc.c:1804)
== by 0x5E357A: ReadRequestFromClient (io.c:336)
== by 0x4A1FAB: Dispatch (dispatch.c:519)
== by 0x4B03B4: dix_main (main.c:277)
== by 0x428941: main (stubmain.c:34)
==
== Invalid write of size 2
== at 0x5A3AD7: CheckKeyTypes (xkb.c:1669)
== by 0x5A6A9C: _XkbSetMapChecks (xkb.c:2515)
== by 0x5A759E: ProcXkbSetMap (xkb.c:2736)
== by 0x5BF832: SProcXkbSetMap (xkbSwap.c:245)
== by 0x5C05ED: SProcXkbDispatch (xkbSwap.c:501)
== by 0x4A20DF: Dispatch (dispatch.c:551)
== by 0x4B03B4: dix_main (main.c:277)
== by 0x428941: main (stubmain.c:34)
== Address is 2 bytes after a block of size 28,672 alloc'd
== at 0x4848897: realloc (vg_replace_malloc.c:1804)
== by 0x5E357A: ReadRequestFromClient (io.c:336)
== by 0x4A1FAB: Dispatch (dispatch.c:519)
== by 0x4B03B4: dix_main (main.c:277)
== by 0x428941: main (stubmain.c:34)
==
To avoid that issue, add additional bounds checking within the loops by
calling _XkbCheckRequestBounds() and report an error if we are to read
past the client's request.
CVE-2026-34003, ZDI-CAN-28736
This vulnerability was discovered by:
Jan-Niklas Sohn working with TrendAI Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit b85b00dd7b)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2178>
As reported by valgrind:
== Conditional jump or move depends on uninitialised value(s)
== at 0x547E5B: CheckModifierMap (xkb.c:1972)
== by 0x54A086: _XkbSetMapChecks (xkb.c:2574)
== by 0x54A845: ProcXkbSetMap (xkb.c:2741)
== by 0x556EF4: ProcXkbDispatch (xkb.c:7048)
== by 0x454A8C: Dispatch (dispatch.c:553)
== by 0x462CEB: dix_main (main.c:274)
== by 0x405EA7: main (stubmain.c:34)
== Uninitialised value was created by a heap allocation
== at 0x4840B26: malloc (vg_replace_malloc.c:447)
== by 0x592D5A: AllocateInputBuffer (io.c:981)
== by 0x591F77: InsertFakeRequest (io.c:516)
== by 0x45CA27: NextAvailableClient (dispatch.c:3629)
== by 0x58FA81: AllocNewConnection (connection.c:628)
== by 0x58FC70: EstablishNewConnections (connection.c:692)
== by 0x58FFAA: HandleNotifyFd (connection.c:809)
== by 0x593F42: ospoll_wait (ospoll.c:660)
== by 0x58B9B6: WaitForSomething (WaitFor.c:208)
== by 0x4548AC: Dispatch (dispatch.c:493)
== by 0x462CEB: dix_main (main.c:274)
== by 0x405EA7: main (stubmain.c:34)
The issue is that the loop in CheckModifierMap() reads from wire without
verifying that the data is within the request bounds.
The req->totalModMapKeys value could exceed the actual data provided,
causing reads of uninitialized memory.
To fix that issue, we add a bounds check using _XkbCheckRequestBounds,
but for that, we need to also pass a ClientPtr parameter, which is not
a problem since CheckModifierMap() is a private, static function.
CVE-2026-34002, ZDI-CAN-28737
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit f056ce1cc9)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2178>
As reported by valgrind:
== Conditional jump or move depends on uninitialised value(s)
== at 0x5CBE66: SrvXkbAddGeomKeyAlias (XKBGAlloc.c:585)
== by 0x5AC7D5: _CheckSetGeom (xkb.c:5607)
== by 0x5AC952: _XkbSetGeometry (xkb.c:5643)
== by 0x5ACB58: ProcXkbSetGeometry (xkb.c:5684)
== by 0x5B0DAC: ProcXkbDispatch (xkb.c:7070)
== by 0x4A28C5: Dispatch (dispatch.c:553)
== by 0x4B0B24: dix_main (main.c:274)
== by 0x42915E: main (stubmain.c:34)
== Uninitialised value was created by a heap allocation
== at 0x4840B26: malloc (vg_replace_malloc.c:447)
== by 0x5E13B0: AllocateInputBuffer (io.c:981)
== by 0x5E05CD: InsertFakeRequest (io.c:516)
== by 0x4AA860: NextAvailableClient (dispatch.c:3629)
== by 0x5DE0D7: AllocNewConnection (connection.c:628)
== by 0x5DE2C6: EstablishNewConnections (connection.c:692)
== by 0x5DE600: HandleNotifyFd (connection.c:809)
== by 0x5E2598: ospoll_wait (ospoll.c:660)
== by 0x5DA00C: WaitForSomething (WaitFor.c:208)
== by 0x4A26E5: Dispatch (dispatch.c:493)
== by 0x4B0B24: dix_main (main.c:274)
== by 0x42915E: main (stubmain.c:34)
Each key alias entry contains two key names (the alias and the real key
name), each of size XkbKeyNameLength.
The current bounds check only validates the first name, allowing
XkbAddGeomKeyAlias to potentially read uninitialized memory when
accessing the second name at &wire[XkbKeyNameLength].
To fix this, change the value to check to use 2 * XkbKeyNameLength to
validate the bounds.
CVE-2026-34000, ZDI-CAN-28679
This vulnerability was discovered by:
Jan-Niklas Sohn working with TrendAI Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 81b6a34f90)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2178>
If the "compat" buffer has previously been truncated, there will be
unused space in the buffer. The code uses this space, but does not
update the number of valid entries in the buffer.
In the best case, this leads to the new compat entries being ignored. In the
worst case, if there are any "skipped" compat entries, the number of
valid entries will be corrupted, potentially leading to a buffer read
overrun when processing a future request.
Set the number of used "compat" entries when re-using previously
allocated space in the buffer.
CVE-2026-33999, ZDI-CAN-28593
This vulnerability was discovered by:
Jan-Niklas Sohn working with TrendAI Zero Day Initiative
Signed-off-by: Peter Harris <pharris2@rocketsoftware.com>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit b024ae1749)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2178>
Before this commit the count of key type level names was wrongly set
in `XkbGetNames`: for key type without names, it was set to the level
count, while it should be 0:
- `XkbComputeGetNamesReplySize()` does not account key type without
level names;
- `XkbSendNames()` does not write any level entry for key types without
level names.
This causes a mismatch offset while parsing the response and its
processing would ultimately fail.
Fixed by setting the correct level name count: 0 if there is no level
name, else the number of levels.
(cherry picked from commit c49cbc176a)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2146>
A key type that has no level names is legit. Before this commit,
`XkbCopyKeymap` would make such level inconsistent by setting its
number of levels to 0 while keeping its map entries. It suffices
to clear the names array.
Fixed by copying the level count from the source type.
WARNING: this will trigger an error in `XkbGetNames`, which worked
before this commit only by chance. This is fixed in the next commit.
(cherry picked from commit 12605989af)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2146>
When a group indicator (or a latched indicator of any kind) is defined,
e.g.:
indicator "Scroll Lock" { groups = Group2; }
the logical and physical indicator state may desync across multiple
connected keyboards.
This is caused by XkbPushLockedStateToSlaves only pushing locked_mods to
the slave devices. Pushing locked_group (as well as latched groups/mods)
along with locked_mods resolves the issue.
The issue is not observed with API calls because a different code path
is taken (avoiding XkbPushLockedStateToSlaves altogether).
Signed-off-by: Alexander Melnyk <inboxnumberzero@zoho.com>
(cherry picked from commit 36a7fdd315)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2146>
The XkbCompatMap structure stores its "num_si" and "size_si" fields
using an unsigned short.
However, the function _XkbSetCompatMap() will store the sum of the
input data "firstSI" and "nSI" in both XkbCompatMap's "num_si" and
"size_si" without first checking if the sum overflows the maximum
unsigned short value, leading to a possible overflow.
To avoid the issue, check whether the sum does not exceed the maximum
unsigned short value, or return a "BadValue" error otherwise.
CVE-2025-62231, ZDI-CAN-27560
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
(cherry picked from commit 475d9f49ac)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2087>
XkbRemoveResourceClient() would free the XkbInterest data associated
with the device, but not the resource associated with it.
As a result, when the client terminates, the resource delete function
gets called and accesses already freed memory:
| Invalid read of size 8
| at 0x5BC0C0: XkbRemoveResourceClient (xkbEvents.c:1047)
| by 0x5B3391: XkbClientGone (xkb.c:7094)
| by 0x4DF138: doFreeResource (resource.c:890)
| by 0x4DFB50: FreeClientResources (resource.c:1156)
| by 0x4A9A59: CloseDownClient (dispatch.c:3550)
| by 0x5E0A53: ClientReady (connection.c:601)
| by 0x5E4FEF: ospoll_wait (ospoll.c:657)
| by 0x5DC834: WaitForSomething (WaitFor.c:206)
| by 0x4A1BA5: Dispatch (dispatch.c:491)
| by 0x4B0070: dix_main (main.c:277)
| by 0x4285E7: main (stubmain.c:34)
| Address 0x1893e278 is 184 bytes inside a block of size 928 free'd
| at 0x4842E43: free (vg_replace_malloc.c:989)
| by 0x49C1A6: CloseDevice (devices.c:1067)
| by 0x49C522: CloseOneDevice (devices.c:1193)
| by 0x49C6E4: RemoveDevice (devices.c:1244)
| by 0x5873D4: remove_master (xichangehierarchy.c:348)
| by 0x587921: ProcXIChangeHierarchy (xichangehierarchy.c:504)
| by 0x579BF1: ProcIDispatch (extinit.c:390)
| by 0x4A1D85: Dispatch (dispatch.c:551)
| by 0x4B0070: dix_main (main.c:277)
| by 0x4285E7: main (stubmain.c:34)
| Block was alloc'd at
| at 0x48473F3: calloc (vg_replace_malloc.c:1675)
| by 0x49A118: AddInputDevice (devices.c:262)
| by 0x4A0E58: AllocDevicePair (devices.c:2846)
| by 0x5866EE: add_master (xichangehierarchy.c:153)
| by 0x5878C2: ProcXIChangeHierarchy (xichangehierarchy.c:493)
| by 0x579BF1: ProcIDispatch (extinit.c:390)
| by 0x4A1D85: Dispatch (dispatch.c:551)
| by 0x4B0070: dix_main (main.c:277)
| by 0x4285E7: main (stubmain.c:34)
To avoid that issue, make sure to free the resources when freeing the
device XkbInterest data.
CVE-2025-62230, ZDI-CAN-27545
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
(cherry picked from commit 10c94238bd)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2087>
Currently, the resource in only available to the xkb.c source file.
In preparation for the next commit, to be able to free the resources
from XkbRemoveResourceClient(), make that variable private instead.
This is related to:
CVE-2025-62230, ZDI-CAN-27545
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
(cherry picked from commit 99790a2c92)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2087>
Handles common case of allocating & copying string to temporary buffer
(cherry picked from xorg/lib/libxkbfile@8a91517ca6ea77633476595b0eb5b213357c60e5)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 42a1f25faf)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1894>
Fixes formatting of negative numbers, so they don't show minus sign
after the decimal point.
(cherry picked from xorg/lib/libxkbfile@d2ec504fec2550f4fd046e801b34317ef4a4bab9)
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 7a23010232)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1894>
Passing a negative value in `needed` to the `XkbResizeKeyActions()`
function can create a `newActs` array of an unespected size.
Check the value and return if it is invalid.
This error has been found by a static analysis tool. This is the report:
Error: OVERRUN (CWE-119):
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: cond_const:
Checking "xkb->server->size_acts == 0" implies that
"xkb->server->size_acts" is 0 on the true branch.
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: buffer_alloc:
"calloc" allocates 8 bytes dictated by parameters
"(size_t)((xkb->server->size_acts == 0) ? 1 : xkb->server->size_acts)"
and "8UL".
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: var_assign:
Assigning: "newActs" = "calloc((size_t)((xkb->server->size_acts == 0) ? 1 : xkb->server->size_acts), 8UL)".
libX11-1.8.7/src/xkb/XKBMAlloc.c:815: assignment:
Assigning: "nActs" = "1".
libX11-1.8.7/src/xkb/XKBMAlloc.c:829: cond_at_least:
Checking "nCopy > 0" implies that "nCopy" is at least 1 on the
true branch.
libX11-1.8.7/src/xkb/XKBMAlloc.c:830: overrun-buffer-arg:
Overrunning buffer pointed to by "&newActs[nActs]" of 8 bytes by
passing it to a function which accesses it at byte offset 15
using argument "nCopy * 8UL" (which evaluates to 8).
# 828|
# 829| if (nCopy > 0)
# 830|-> memcpy(&newActs[nActs], XkbKeyActionsPtr(xkb, i),
# 831| nCopy * sizeof(XkbAction));
# 832| if (nCopy < nKeyActs)
(cherry picked from xorg/lib/libx11@af1312d2873d2ce49b18708a5029895aed477392)
Signed-off-by: José Expósito <jexposit@redhat.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 6d33834186)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1894>
If there was a previous radio_groups array which we failed to realloc
and freed instead, clear the array size in the XkbNamesRec.
Taken from xorg/lib/libx11@258a8ced681dc1bc50396be7439fce23f9807e2a
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 09c6f09eb7)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1894>
If XkbChangeTypesOfKey() is called with nGroups == 0, it will resize the
key syms to 0 but leave the key actions unchanged.
If later, the same function is called with a non-zero value for nGroups,
this will cause a buffer overflow because the key actions are of the wrong
size.
To avoid the issue, make sure to resize both the key syms and key actions
when nGroups is 0.
CVE-2025-26597, ZDI-CAN-25683
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 0e4ed94952)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1830>
The computation of the length in XkbSizeKeySyms() differs from what is
actually written in XkbWriteKeySyms(), leading to a heap overflow.
Fix the calculation in XkbSizeKeySyms() to match what kbWriteKeySyms()
does.
CVE-2025-26596, ZDI-CAN-25543
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 80d69f0142)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1830>
The code in XkbVModMaskText() allocates a fixed sized buffer on the
stack and copies the virtual mod name.
There's actually two issues in the code that can lead to a buffer
overflow.
First, the bound check mixes pointers and integers using misplaced
parenthesis, defeating the bound check.
But even though, if the check fails, the data is still copied, so the
stack overflow will occur regardless.
Change the logic to skip the copy entirely if the bound check fails.
CVE-2025-26595, ZDI-CAN-25545
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 11fcda8753)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1830>
Generating the modifier modmap, the helper function generate_modkeymap()
would check the entire range up to the MAP_LENGTH.
However, the given keymap might have less keycodes than MAP_LENGTH, in
which case we would go beyond the size of the modmap, as reported by
ASAN:
==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1 at 0x5110001c225b thread T0
#0 0x5e7369393873 in generate_modkeymap ../dix/inpututils.c:309
#1 0x5e736930dcce in ProcGetModifierMapping ../dix/devices.c:1794
#2 0x5e7369336489 in Dispatch ../dix/dispatch.c:550
#3 0x5e736934407d in dix_main ../dix/main.c:275
#5 0x7e46d47b2ecb in __libc_start_main
#6 0x5e73691be324 in _start (xserver/build/hw/xwayland/Xwayland)
Address is located 0 bytes after 219-byte region
allocated by thread T0 here:
#0 0x7e46d4cfc542 in realloc
#1 0x5e73695aa90e in _XkbCopyClientMap ../xkb/xkbUtils.c:1142
#2 0x5e73695aa90e in XkbCopyKeymap ../xkb/xkbUtils.c:1966
#3 0x5e73695b1b2f in XkbDeviceApplyKeymap ../xkb/xkbUtils.c:2023
#4 0x5e73691c6c18 in keyboard_handle_keymap ../hw/xwayland/xwayland-input.c:1194
As MAP_LENGTH is used in various code paths where the max keycode might
not be easily available, best is to always use MAP_LENGTH to allocate the
keymaps so that the code never run past the buffer size.
If the max key code is smaller than the MAP_LENGTH limit, fill-in the gap
with zeros.
That also simplifies the code slightly as we do not constantly need to
reallocate the keymap to adjust to the max key code size.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1780
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 92bcebfd7e)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1767>
The _XkbSetCompatMap() function attempts to resize the `sym_interpret`
buffer.
However, It didn't update its size properly. It updated `num_si` only,
without updating `size_si`.
This may lead to local privilege escalation if the server is run as root
or remote code execution (e.g. x11 over ssh).
CVE-2024-9632, ZDI-CAN-24756
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: José Expósito <jexposit@redhat.com>
(cherry picked from commit 85b7765714)
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1734>
GetComponentByName returns an allocated string, so let's free that if we
fail somewhere.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 18f91b950e)
GetCountedString did a check for the whole string to be within the
request buffer but not for the initial 2 bytes that contain the length
field. A swapped client could send a malformed request to trigger a
swaps() on those bytes, writing into random memory.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 11beef0b7f)
Unlike other elements of the keymap, this pointer was freed but not
reset. On a subsequent XkbGetKbdByName request, the server may access
already freed memory.
CVE-2022-4283, ZDI-CAN-19530
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit ccdd431cd8)
No validation of the various fields on that report were done, so a
malicious client could send a short request that claims it had N
sections, or rows, or keys, and the server would process the request for
N sections, running out of bounds of the actual request data.
Fix this by adding size checks to ensure our data is valid.
ZDI-CAN 16062, CVE-2022-2319.
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 6907b6ea2b)
XKB often uses a FooCheck and Foo function pair, the former is supposed
to check all values in the request and error out on BadLength,
BadValue, etc. The latter is then called once we're confident the values
are good (they may still fail on an individual device, but that's a
different topic).
In the case of XkbSetDeviceInfo, those functions were incorrectly
named, with XkbSetDeviceInfo ending up as the checker function and
XkbSetDeviceInfoCheck as the setter function. As a result, the setter
function was called before the checker function, accessing request
data and modifying device state before we ensured that the data is
valid.
In particular, the setter function relied on values being already
byte-swapped. This in turn could lead to potential OOB memory access.
Fix this by correctly naming the functions and moving the length checks
over to the checker function. These were added in 87c64fc5b0 to the
wrong function, probably due to the incorrect naming.
Fixes ZDI-CAN 16070, CVE-2022-2320.
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Introduced in c06e27b2f6
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit dd8caf39e9)
Most similar loops here use a pointer that advances with each loop
iteration, let's do the same here for consistency.
No functional changes.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit f1070c01d6)
As the comment says:
"symsPerKey/mapWidths must be filled regardless of client-side flags"
so we always have to call CheckKeyTypes which will notably fill mapWidths
and nTypes. That is needed for CheckKeySyms to work since it checks the
width. Without it, any request with XkbKeySymsMask but not
XkbKeyTypesMask will fail because of the missing width information, for
instance this:
XkbDescPtr xkb;
if (!(xkb = XkbGetMap (dpy, XkbKeyTypesMask|XkbKeySymsMask, XkbUseCoreKbd))) {
fprintf (stderr, "ERROR getting map\n");
exit(1);
}
XFlush (dpy);
XSync (dpy, False);
XkbMapChangesRec changes = { .changed = 0 };
int oneGroupType[XkbNumKbdGroups] = { XkbOneLevelIndex };
if (XkbChangeTypesOfKey(xkb, keycode, 1, XkbGroup1Mask, oneGroupType, &changes)) {
fprintf(stderr, "ERROR changing type of key\n");
exit(1);
}
XkbKeySymEntry(xkb,keycode,0,0) = keysym;
if (!XkbChangeMap(dpy,xkb,&changes)) {
fprintf(stderr, "ERROR changing map\n");
exit(1);
}
XkbFreeKeyboard (xkb, 0, TRUE);
XFlush (dpy);
XSync (dpy, False);
This had being going under the radar since about ever until commit
de940e06f8 ("xkb: fix key type index check
in _XkbSetMapChecks") fixed checking the values of kt_index, which was
previously erroneously ignoring errors and ignoring all other checks, just
because nTypes was not set, precisely because CheckKeyTypes was not called.
Note: yes, CheckKeyTypes is meant to be callable without XkbKeyTypesMask, it
does properly check for that and just fills nTypes and mapWidths in that
case.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
(cherry picked from commit 0217cc6e0c)
The previous if/else condition resulted in us always setting the key
type count to the current number of key types. Split this up correctly.
Regression introduced in de940e06f8Fixes#1249
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit be16bd8543)
Commit 446ff2d317 added checks to
prevalidate the size of incoming SetMap requests.
That commit checks for the XkbSetMapResizeTypes flag to be set before
allowing key types data to be processed.
key types data can be changed or even just sent wholesale unchanged
without the number of key types changing, however. The check for
XkbSetMapResizeTypes rejects those legitimate requests. In particular,
XkbChangeMap never sets XkbSetMapResizeTypes and so always fails now
any time XkbKeyTypesMask is in the changed mask.
This commit drops the check for XkbSetMapResizeTypes in flags when
prevalidating the request length.
(cherry picked from commit 8b7f4d3259)
I get this:
In function ‘TryCopyStr’,
inlined from ‘CopyISOLockArgs’ at ../xkb/xkbtext.c:875:9:
../xkb/xkbtext.c:720:13: warning: ‘tbuf’ may be used uninitialized [-Wmaybe-uninitialized]
720 | strcat(to, from);
| ^~~~~~~~~~~~~~~~
../xkb/xkbtext.c: In function ‘CopyISOLockArgs’:
<built-in>: note: by argument 1 of type ‘const char *’ to ‘__builtin_strlen’ declared here
../xkb/xkbtext.c:871:10: note: ‘tbuf’ declared here
871 | char tbuf[64];
| ^~~~
Just initialize tbuf so it definitely works.
xkb.c: In function ‘ProcXkbSetMap’:
xkb.c:2747:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
2747 | DeviceIntPtr master = GetMaster(dev, MASTER_KEYBOARD);
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Rework the function to use a single snprintf call instead of a mix of
strcpy/strcats. This now also appends a trailing slash where needed so we
don't rely on the build system to set this for us.
Also, since /tmp/ is the fallback and we never check if everything succeeded,
assert if we can't use /tmp/. This will never be triggered anyway, the only
caller to OutputDirectory() uses sizeof(PATH_MAX-sized array).
Follow-up from 6c51818a0f
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The xkb spec defines that the 7th element of the DeviceValuator key
action description is "valuator 2 value".
This error most likely was accidentally introduced as a copy-paste error
in edeb033f29.
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
Calling strlen on char[4] that does not need to contain '\0' is wrong and X
server may end up running into uninitialized memory.
In addition GCC 8 is clever enough that it knows that strlen on char[4] can
return 0, 1, 2, 3 or cause undefined behavior. With this knowledge it can
optimize away the min(..., 4). In reality it can cause the memcpy to be called
with bigger size than 4 and overflow the destination buffer.
Fixes: 83913de25d (xkb: Silence some compiler warnings)
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/288
Signed-off-by: Matt Turner <mattst88@gmail.com>
Avoid out of bounds memory accesses on too short request.
ZDI-CAN 11572 / CVE-2020-14360
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
ZDI-CAN 11389 / CVE-2020-25712
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Resolves warning from Oracle Parfait static analyser:
Error: Uninitialised memory
Uninitialised memory variable [uninitialised-mem-var] (CWE 457):
Possible access to uninitialised memory referenced by variable 'mask'
at line 721 of xkb/XKBMisc.c in function 'XkbUpdateKeyTypeVirtualMods'.
Path in callee avoiding write at line 720
mask allocated at line 718
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
