[PATCH] ui/clipboard: avoid crash upon request when clipboard peer is not initialized

[Fiona Ebner <f.ebner@proxmox.com>]

With VNC, it can be that a client sends a VNC_MSG_CLIENT_CUT_TEXT
message before sending a VNC_MSG_CLIENT_SET_ENCODINGS message with
VNC_ENCODING_CLIPBOARD_EXT for configuring the clipboard extension.

This means that qemu_clipboard_request() can be reached (via
vnc_client_cut_text_ext()) before vnc_server_cut_text_caps() was
called and had the chance to initialize the clipboard peer. In that
case, info->owner->request is NULL instead of a function and so
attempting to call it in qemu_clipboard_request() results in a
segfault.

In particular, this can happen when using the KRDC (22.12.3) VNC
client on Wayland.

It is not enough to check in ui/vnc.c's protocol_client_msg() if the
VNC_FEATURE_CLIPBOARD_EXT feature is enabled before handling an
extended clipboard message with vnc_client_cut_text_ext(), because of
the following scenario with two clients (say noVNC and KRDC):

The noVNC client sets the extension VNC_FEATURE_CLIPBOARD_EXT and
initializes its cbpeer.

The KRDC client does not, but triggers a vnc_client_cut_text() (note
it's not the _ext variant)). There, a new clipboard info with it as
the 'owner' is created and via qemu_clipboard_set_data() is called,
which in turn calls qemu_clipboard_update() with that info.

In qemu_clipboard_update(), the notifier for the noVNC client will be
called, i.e. vnc_clipboard_notify() and also set vs->cbinfo for the
noVNC client. The 'owner' in that clipboard info is the clipboard peer
for the KRDC client, which did not initialize the 'request' function.
That sounds correct to me, it is the owner of that clipboard info.

Then when noVNC sends a VNC_MSG_CLIENT_CUT_TEXT message (it did set
the feature correctly, so the check added by your patch passes), that
clipboard info is passed to qemu_clipboard_request() and the original
segfault still happens.

Fixes: CVE-2023-6683
Reported-by: Markus Frank <m.frank@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Markus Frank <m.frank@proxmox.com>
---

This is just a minimal fix. Happy to add some warning/error to not
hide the issue with the missing initialization completely and/or go
for a different approach with a check somewhere in the VNC code.

 ui/clipboard.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ui/clipboard.c b/ui/clipboard.c
index 3d14bffaf8..c13b54d2e9 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info,
     if (info->types[type].data ||
         info->types[type].requested ||
         !info->types[type].available ||
-        !info->owner)
+        !info->owner ||
+        !info->owner->request)
         return;
 
     info->types[type].requested = true;
-- 
2.39.2