Fix out-of-bounds copy in xcb_send_event() calls

xcb_send_event always copies 32 bytes, so we have to pad all xcb_*_event_t
to 32 bytes to avoid leaking uninitialized stack memory. I found this
problem while running kwin_x11 on a CHERI-RISC-V system (which has bounded
pointers). The xcb_send_event() implementation has a memcpy() that was
copying 32 bytes but the event passed was a bounded to 28 bytes, so this
resulted in a run-time exception in X11Client::sendClientMessage().
The same problem exists in Selection::sendSelectionNotify(), but this time
we could end up copying up to 8 bytes since xcb_selection_notify_event_t
is only 24 bytes.

This disclosure of uninitialized data could in theory have a security
impact if it leaks a pointer value (e.g. a return address) as part of an
exploit chain that needs to bypass ASLR. However, the selection notify
events go directly to the XServer and you most likely already have a
serious problem if an attacker has full control over the XServer. It is
possible that the configure notify events go directly to an untrusted
client, but even if they do this leak is not directly exploitable.

See also https://gitlab.freedesktop.org/xorg/lib/libxcb/-/issues/18

src/x11client.cpp

@@ -1745,6 +1745,10 @@ void X11Client::updateHiddenPreview()
 void X11Client::sendClientMessage(xcb_window_t w, xcb_atom_t a, xcb_atom_t protocol, uint32_t data1, uint32_t data2, uint32_t data3)
 {
     xcb_client_message_event_t ev;
+    // Every X11 event is 32 bytes (see man xcb_send_event), so XCB will copy
+    // 32 unconditionally. Add a static_assert to ensure we don't disclose
+    // stack memory.
+    static_assert(sizeof(ev) == 32, "Would leak stack data otherwise");
     memset(&ev, 0, sizeof(ev));
     ev.response_type = XCB_CLIENT_MESSAGE;
     ev.window = w;
@@ -3650,19 +3654,26 @@ QSize X11Client::basicUnit() const
  */
 void X11Client::sendSyntheticConfigureNotify()
 {
-    xcb_configure_notify_event_t c;
-    memset(&c, 0, sizeof(c));
-    c.response_type = XCB_CONFIGURE_NOTIFY;
-    c.event = window();
-    c.window = window();
-    c.x = m_clientGeometry.x();
-    c.y = m_clientGeometry.y();
-    c.width = m_clientGeometry.width();
-    c.height = m_clientGeometry.height();
-    c.border_width = 0;
-    c.above_sibling = XCB_WINDOW_NONE;
-    c.override_redirect = 0;
-    xcb_send_event(connection(), true, c.event, XCB_EVENT_MASK_STRUCTURE_NOTIFY, reinterpret_cast<const char*>(&c));
+    // Every X11 event is 32 bytes (see man xcb_send_event), so XCB will copy
+    // 32 unconditionally. Use a union to ensure we don't disclose stack memory.
+    union {
+        xcb_configure_notify_event_t event;
+        char buffer[32];
+    } u;
+    static_assert(sizeof(u.event) < 32, "wouldn't need the union otherwise");
+    memset(&u, 0, sizeof(u));
+    xcb_configure_notify_event_t &c = u.event;
+    u.event.response_type = XCB_CONFIGURE_NOTIFY;
+    u.event.event = window();
+    u.event.window = window();
+    u.event.x = m_clientGeometry.x();
+    u.event.y = m_clientGeometry.y();
+    u.event.width = m_clientGeometry.width();
+    u.event.height = m_clientGeometry.height();
+    u.event.border_width = 0;
+    u.event.above_sibling = XCB_WINDOW_NONE;
+    u.event.override_redirect = 0;
+    xcb_send_event(connection(), true, c.event, XCB_EVENT_MASK_STRUCTURE_NOTIFY, reinterpret_cast<const char*>(&u));
     xcb_flush(connection());
 }
 

src/xwl/drag.cpp

@@ -35,6 +35,7 @@ void Drag::sendClientMessage(xcb_window_t target, xcb_atom_t type, xcb_client_me
         type,       // type
         *data,      // data
     };
+    static_assert(sizeof(ev) == 32, "Would leak stack data otherwise");
 
     xcb_connection_t *xcbConn = kwinApp()->x11Connection();
     xcb_send_event(xcbConn,

src/xwl/selection.cpp

@@ -134,21 +134,24 @@ bool Selection::filterEvent(xcb_generic_event_t *event)
 
 void Selection::sendSelectionNotify(xcb_selection_request_event_t *event, bool success)
 {
+    // Every X11 event is 32 bytes (see man xcb_send_event), so XCB will copy
+    // 32 unconditionally. Use a union to ensure we don't disclose stack memory.
+    union {
         xcb_selection_notify_event_t notify;
-    notify.response_type = XCB_SELECTION_NOTIFY;
-    notify.sequence = 0;
-    notify.time = event->time;
-    notify.requestor = event->requestor;
-    notify.selection = event->selection;
-    notify.target = event->target;
-    notify.property = success ? event->property : xcb_atom_t(XCB_ATOM_NONE);
+        char buffer[32];
+    } u;
+    memset(&u, 0, sizeof(u));
+    static_assert(sizeof(u.notify) < 32, "wouldn't need the union otherwise");
+    u.notify.response_type = XCB_SELECTION_NOTIFY;
+    u.notify.sequence = 0;
+    u.notify.time = event->time;
+    u.notify.requestor = event->requestor;
+    u.notify.selection = event->selection;
+    u.notify.target = event->target;
+    u.notify.property = success ? event->property : xcb_atom_t(XCB_ATOM_NONE);
 
     xcb_connection_t *xcbConn = kwinApp()->x11Connection();
-    xcb_send_event(xcbConn,
-                   0,
-                   event->requestor,
-                   XCB_EVENT_MASK_NO_EVENT,
-                   (const char *)&notify);
+    xcb_send_event(xcbConn, 0, event->requestor, XCB_EVENT_MASK_NO_EVENT, (const char *)&u);
     xcb_flush(xcbConn);
 }