Commit graph

632 commits

Author SHA1 Message Date
Olivier Fourdan
c7beaec76c xkb: Prevent overflow in XkbSetCompatMap()
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/2088>
2025-10-28 14:27:03 +01:00
Olivier Fourdan
1abca0b9b5 xkb: Free the XKB resource when freeing XkbInterest
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/2088>
2025-10-28 14:27:03 +01:00
Olivier Fourdan
a1d4f04bbd xkb: Make the RT_XKBCLIENT resource private
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/2088>
2025-10-28 14:27:03 +01:00
Olivier Fourdan
dfb1d1e202 Revert "xwayland: Don't run key behaviors and actions"
This breaks modifiers in gamescope.

This reverts commit 482623371f.

See-also: https://github.com/ValveSoftware/gamescope/issues/1740
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2081>
2025-10-15 15:11:13 +02:00
Julian Orth
482623371f xwayland: Don't run key behaviors and actions
Consider the following keymap:

```xkb
xkb_keymap {
    xkb_keycodes {
        <compose> = 135;
    };
    xkb_symbols {
        key <compose> {
            [ SetGroup(group = +1) ]
        };
    };
};
```

When the user presses the compose key, the following happens:

1. The compositor forwards the key to Xwayland.
2. Xwayland executes the SetGroup action and sets the base_group to 1
   and the effective group to 1.
3. The compositor updates its own state and sends the effective group,
   1, to Xwayland.
4. Xwayland sets the locked group to 1 and the effective group to
   1 + 1 = 2.

This is wrong since pressing compose should set the effective group to 1
but to X applications the effective group appears to be 2.

This commit makes it so that Xwayland completely ignores the key
behaviors and actions of the keymap and only updates the modifier and
group components in response to the wayland modifiers events.

Signed-off-by: Julian Orth <ju.orth@gmail.com>
(cherry picked from commit 45c1d22ff6)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2060>
2025-09-09 12:01:42 +02:00
Alan Coopersmith
91e42e523c xkb: Add tbGetBufferString helper function
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/1895>
2025-04-08 10:03:55 +02:00
Martin Burggraf
bd43d90cd1 xkb: correcting mathematical nonsense in XkbGeomFPText
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/1895>
2025-04-08 10:03:50 +02:00
Alan Coopersmith
ed54cc2a03 xkb: Convert more sprintf calls to snprintf in xkbtext.c
Based on xorg/lib/libxkbfile@390acfe5bb88cdab509b5eaae4041f265e969d2b

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 60419d8e4a)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1895>
2025-04-08 10:03:45 +02:00
José Expósito
b32d637f51 xkb: Check that needed is > 0 in XkbResizeKeyActions
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/1895>
2025-04-08 10:03:40 +02:00
Alan Coopersmith
852595a51d xkb: ensure XkbAllocNames sets num_rg to 0 on allocation failure
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/1895>
2025-04-08 10:03:35 +02:00
Olivier Fourdan
39c7ed837b xkb: Fix buffer overflow in XkbChangeTypesOfKey()
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/1831>
2025-02-25 19:38:11 +01:00
Olivier Fourdan
60df821467 xkb: Fix computation of XkbSizeKeySyms
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/1831>
2025-02-25 19:38:11 +01:00
Olivier Fourdan
bfbb53e0b9 xkb: Fix buffer overflow in XkbVModMaskText()
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/1831>
2025-02-25 19:38:11 +01:00
Olivier Fourdan
f04096af3c Revert "xwayland: Don't run key behaviors and actions"
Due to a bug in gamescope who does not send the Wayland modifiers
events, this is causing a regression in X11 clients running on Xwayland
in gamescope, where the modifiers are not applied anymore.

This reverts commit cea92a3e09.

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1790
See-also: https://github.com/ValveSoftware/gamescope/issues/1740
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1781>
2025-02-10 15:43:41 +01:00
Julian Orth
cea92a3e09 xwayland: Don't run key behaviors and actions
Consider the following keymap:

```xkb
xkb_keymap {
    xkb_keycodes {
        <compose> = 135;
    };
    xkb_symbols {
        key <compose> {
            [ SetGroup(group = +1) ]
        };
    };
};
```

When the user presses the compose key, the following happens:

1. The compositor forwards the key to Xwayland.
2. Xwayland executes the SetGroup action and sets the base_group to 1
   and the effective group to 1.
3. The compositor updates its own state and sends the effective group,
   1, to Xwayland.
4. Xwayland sets the locked group to 1 and the effective group to
   1 + 1 = 2.

This is wrong since pressing compose should set the effective group to 1
but to X applications the effective group appears to be 2.

This commit makes it so that Xwayland completely ignores the key
behaviors and actions of the keymap and only updates the modifier and
group components in response to the wayland modifiers events.

Signed-off-by: Julian Orth <ju.orth@gmail.com>
(cherry picked from commit 45c1d22ff6)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1766>
2025-02-04 18:14:06 +01:00
Olivier Fourdan
ffd4d2a268 xkb: Always use MAP_LENGTH keymap size
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/1766>
2025-02-04 09:21:04 +01:00
Matthieu Herrb
26120df7aa xkb: Fix buffer overflow in _XkbSetCompatMap()
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/1735>
2024-10-29 16:26:59 +01:00
Enrico Weigelt, metux IT consult
beb073813e xkb: drop ununsed XkbNameMatchesPattern()
Not used anywhere, also not in drivers, so we can drop it.
Probably never been used, just added accidentially back in 2008.

Fixes: 68bd7ac193
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1477>
2024-04-09 06:56:20 +00:00
Enrico Weigelt, metux IT consult
fdea36708c xkb: drop never used XkmProbe()
This function was (accidentally ?) added back in 2008, but never used.

Fixes: 68bd7ac193
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1472>
2024-04-09 06:35:05 +00:00
Enrico Weigelt, metux IT consult
95faea8880 xkb: make XkbUpdateKeyTypesFromCore() static
This function is only used inside the same .c file where it's defined,
no outside users, also not in drivers. Thus no need to keep it exported.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1468>
2024-04-03 19:54:51 +02:00
Enrico Weigelt, metux IT consult
30b403b0f0 os: unexport Fopen(), Fclose(), Popen(), Pclose(), System()
These functions are just used for reading auth file or calling xkbcomp while
dropping privileges, in case the Xserver is started as unprivileged user
with suid-root. Thus, shouldn't be used (and aren't used) by drivers.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1370>
2024-03-09 17:18:46 +00:00
Enrico Weigelt, metux IT consult
86ea30733f xkb: drop duplicate _X_EXPORT from .c source
It's already defined in input.h, and that's where it belongs.
(we see from the header, which symbols belong to the module api)

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1274>
2024-03-03 22:34:26 +00:00
Peter Hutterer
eaa92ea422 Revert "include: move BUG_*() macros to separate header"
This breaks the xf86-input-synaptics driver:

  synaptics.c: In function 'clickpad_guess_clickfingers':
  synaptics.c:2638:5: error: implicit declaration of function 'BUG_RETURN_VAL' [-Werror=implicit-function-declaration]
   2638 |     BUG_RETURN_VAL(hw->num_mt_mask > sizeof(close_point) * 8, 0);

This reverts commit 442aec2219.

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1316>
2024-02-23 23:11:01 +00:00
Enrico Weigelt, metux IT consult
bc90c44e60 xkb: fix int size mismatch
GCC reports:

../xkb/xkb.c: In function ‘_XkbSetMapCheckLength’:
../xkb/xkb.c:2464:54: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 2 has type ‘size_t’ {aka ‘unsigned int’} [-Wformat=]
 2464 |     ErrorF("[xkb] BOGUS LENGTH in SetMap: expected %ld got %ld\n",
      |                                                    ~~^
      |                                                      |
      |                                                      long int
      |                                                    %d
 2465 |            len, req_len);
      |            ~~~
      |            |
      |            size_t {aka unsigned int}
../xkb/xkb.c:2464:62: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘size_t’ {aka ‘unsigned int’} [-Wformat=]
 2464 |     ErrorF("[xkb] BOGUS LENGTH in SetMap: expected %ld got %ld\n",
      |                                                            ~~^
      |                                                              |
      |                                                              long int
      |                                                            %d
 2465 |            len, req_len);
      |                 ~~~~~~~
      |                 |
      |                 size_t {aka unsigned int}

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1257>
2024-02-22 23:56:37 +00:00
Enrico Weigelt, metux IT consult
374ee7acd7 xkb: drop defining XKBSRV_NEED_FILE_FUNCS
No need to define XKBSRV_NEED_FILE_FUNCS, for about 15 years now
(since XKBsrv.h isn't used anymore), so drop it.

Fixes: e5f002edde
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
2024-02-19 00:44:15 +00:00
Enrico Weigelt
442aec2219 include: move BUG_*() macros to separate header
Yet another step of uncluttering includes: move out the BUG_* macros
into a separate header, which then is included as-needed.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
2024-02-15 23:33:46 +00:00
Peter Hutterer
073b90ea56 xkb: free the filters
Direct leak of 960 byte(s) in 6 object(s) allocated from:
    #0 0x7f00a4ed8cc7 in calloc (/lib64/libasan.so.8+0xd8cc7) (BuildId: 6f17f87dc4c1aa9f9dde7c4856604c3a25ba4872)
    #1 0x994944 in _XkbNextFreeFilter ../xkb/xkbActions.c:1142
    #2 0x9971b3 in SrvXkbLatchModifiers ../xkb/xkbActions.c:1452
    #3 0x41dec7 in keyboard_handle_modifiers ../hw/xwayland/xwayland-input.c:1211
    #4 0x7f00a4145055 in ffi_call_unix64 (/lib64/libffi.so.8+0x9055) (BuildId: 308041eea4a8d89d9265d3c24b7261dfbe44a61e)

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2023-12-05 14:21:32 +10:00
Alessandro Bono
bddcaf0886 ddxLoad: Check XDG_RUNTIME_DIR before fallback to /tmp/
The XKM_OUTPUT_DIR folder by default is defined as ${datadir}/X11/xkb/compiled
and it is usually defined as /var/lib/xkb or %{_localstatedir}/lib/xkb by
distributions. If X is executed as non-root it won't have permissions to write
into that folder. If we fallback directly to /tmp we might get name collisions:
```
> Error: Cannot open "/tmp/server-10.xkm" to write keyboard description
> Exiting
```

Where the file /tmp/server-10.xkm already exists but is owned by another user
that previously executed X and had the display number 10. This is specially
problematic when exeuting Xvfb.

Before falling back to /tmp/ check first the XDG_RUNTIME_DIR.
2023-04-06 00:48:12 +00:00
Benno Schulenberg
6153c71cfb xkbUtils: use existing symbol names instead of deleted deprecated ones
Symbols `XK_Cyrillic_DZHE` and `XK_Serbian_DZE` were pure synonyms.
2023-03-27 20:11:55 +02:00
Yao Wei
7ce57e179b dix: Force update LEDs after device state update in EnableDevice
This is to make sure the hardware gets the device states regardless
whether the internal state has changed or not, to overcome situations
that device LEDs are out of sync e.g. switching between VTs.

Signed-off-by: Yao Wei (魏銘廷) <yao.wei@canonical.com>
2023-02-21 03:43:05 +00:00
Peter Hutterer
ccdd431cd8 xkb: reset the radio_groups pointer to NULL after freeing it
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>
2022-12-14 11:02:40 +10:00
FeepingCreature
79c572fbd3 xkb: Avoid length-check failure on empty strings.
_XkbCheckRequestBounds assumes that from..to is at least one byte.
However, request strings can be empty, causing spurious failures in
XkbGetKbdByName calls. To avoid this, before checking bounds make
sure that the length is nonzero.
2022-08-31 06:38:54 +00:00
Peter Hutterer
11beef0b7f xkb: proof GetCountedString against request length attacks
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>
2022-07-13 14:37:51 +10:00
Peter Hutterer
1bb7767f19 xkb: length-check XkbListComponents before accessing the fields
Each string length field was accessed before checking whether that byte
was actually part of the client request. No real harm here since it
would immediately fail with BadLength anyway, but let's be correct here.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2022-07-13 11:38:16 +10:00
Peter Hutterer
44ae6f4419 xkb: length-check XkbGetKbdByName before accessing the fields
This request accessed &stuff[1] before length-checking everything. The
check was performed afterwards so invalid requests would return
BadLength anyway, but let's do this before we actually access the
memory.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2022-07-13 11:33:00 +10:00
Peter Hutterer
18f91b950e xkb: fix some possible memleaks in XkbGetKbdByName
GetComponentByName returns an allocated string, so let's free that if we
fail somewhere.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2022-07-13 11:32:07 +10:00
Peter Hutterer
6907b6ea2b xkb: add request length validation for XkbSetGeometry
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>
2022-07-12 15:19:27 +03:00
Peter Hutterer
dd8caf39e9 xkb: swap XkbSetDeviceInfo and XkbSetDeviceInfoCheck
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>
2022-07-12 15:18:52 +03:00
Peter Hutterer
f1070c01d6 xkb: switch to array index loops to moving pointers
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>
2022-07-08 14:27:04 +00:00
Peter Hutterer
6f234c60d9 xkb: whitespace fixes
Sick of fighting vim and git from trying to add this fix with every
commit iteration...

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2022-07-08 14:27:04 +00:00
Peter Hutterer
a2762514b6 xkb: rename xkb.h to xkb-procs.h
This header merely defines the various protocol request handlers, so
let's rename it to something less generic and remove its include from
all the files that don't actually need it (which is almost all of them).

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2022-07-08 14:27:04 +00:00
Peter Hutterer
d9dd300b8d xkb: move the SProcXkbDispatch declaration
Let's move this to where all the other protocol handlers are.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2022-07-08 14:27:04 +00:00
Konstantin Kharlamov
bad94e88ce xkbtext: fix copy-paste error
As can be seen in diff, nOut is always 0 here. The code was likely
copy-pasted from comparisons further below.

Fixes LGTM warning "Comparison is always false because nOut <= 0."

Signed-off-by: Konstantin Kharlamov <Hi-Angel@yandex.ru>
2022-04-02 18:06:30 +00:00
Samuel Thibault
0217cc6e0c xkb: fix XkbSetMap when changing a keysym without changing a keytype
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>
2022-02-03 21:44:37 +00:00
Peter Hutterer
be16bd8543 xkb: fix XkbSetMap check for the keytypes count
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 de940e06f8

Fixes #1249

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2021-12-03 00:33:23 +00:00
Povilas Kanapickas
c97397dc47 Remove autotools support
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
2021-10-27 13:15:40 +03:00
Ray Strode
8b7f4d3259 xkb: Drop check for XkbSetMapResizeTypes
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.
2021-10-07 14:17:26 +00:00
Adam Jackson
b49f0f9b32 xkb: Silence a warning from gcc 11
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.
2021-08-17 16:04:40 -04:00
Peter Hutterer
66ce61983d xkb: silence a compiler warning
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>
2021-04-09 17:37:29 +00:00
Peter Hutterer
f6b8f8c071 xkb: don't require a trailing slash for the XKM output dir
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>
2021-04-09 17:37:29 +00:00