GUACAMOLE-2263: Harden CLIPRDR channel against FreeRDP 3.x clipboard state machine. by escra · Pull Request #661 · apache/guacamole-server

escra · 2026-04-14T21:10:09Z

Summary

Six fixes for the clipboard redirection (CLIPRDR) plugin that prevent RDP
session disconnects when clipboard operations are performed with FreeRDP 3.x
clients.

FreeRDP 3.x has a significantly stricter CLIPRDR state machine than 2.x.
Several patterns that were harmless with FreeRDP 2.x now cause session
teardowns:

Overlapping Format Data Requests - FreeRDP 3.x tracks pending requests
internally and disconnects with error 1359 if a second request arrives
before the first is answered. Fix: add request_pending flag with
request_lock mutex.
CB_RESPONSE_FAIL not checked - If the server responds with a failure
flag, the old code would attempt to process the (NULL/garbage) data,
causing a crash. Fix: check msgFlags & CB_RESPONSE_FAIL early.
No response for unsupported formats - When the server requests an
unsupported clipboard format, the old code silently drops the request.
FreeRDP 3.x times out after 10 seconds and tears down the session. Fix:
send CB_RESPONSE_FAIL for unsupported formats.
NULL/empty data guard - requestedFormatData can be NULL or
dataLen can be zero in edge cases. Fix: guard before processing.
Stale pending requests - When a new Format List arrives (server
clipboard content changed), any pending request for old data becomes
stale. Fix: clear request_pending when a new Format List is received.
Non-OK return codes treated as fatal - FreeRDP 3.x treats non-OK
returns from CLIPRDR callbacks as fatal channel errors. Fix: all callbacks
now return CHANNEL_RC_OK and log errors instead.

Affected versions

staging/1.6.1 - with FreeRDP 3.x (default in Docker image)
main - same patterns exist

Test plan

Connect from FreeRDP 3.x client, perform rapid copy/paste operations
Copy unsupported clipboard format (e.g., image) on server, verify no disconnect
Verify clipboard works normally with FreeRDP 2.x (no regression)
Stress test: rapid clipboard changes while switching windows
Verify no error 1359 disconnects under load

Five fixes for the clipboard redirection (CLIPRDR) plugin that prevent RDP session disconnects when clipboard operations are performed with FreeRDP 3.x: 1. Add request_pending flag with request_lock mutex to prevent overlapping Format Data Requests that desync FreeRDP's internal state tracking (error 1359). 2. Check CB_RESPONSE_FAIL in format_data_response before processing data to prevent NULL pointer dereference on failed server responses. 3. Send FAIL response for unsupported formats in format_data_request instead of silently dropping — FreeRDP 3.x times out and tears down the session if no response arrives. 4. Guard against NULL or empty requestedFormatData in format_data_response to prevent crash on malformed PDUs. 5. Clear stale pending requests when a new Format List arrives. 6. All CLIPRDR callbacks now always return CHANNEL_RC_OK — FreeRDP 3.x treats non-OK returns as fatal channel errors that disconnect the entire session. Related: FreeRDP/FreeRDP#11847

necouchman

The code itself looks pretty solid, but I have a few things to clear up and/or change. Overall:

I'm concerned that several of the changes, here, overlap with #659, particularly as it relates to allocation of clipboard space. If I'm wrong about that, just let me know, but otherwise please keep those changes distinct.
You've removed comments from the code in several places, with no clear indication as to why. I've tried to put my review comments at each of those places, but please leave comments in-line with the code, unless there's a specific reason they should be removed. If they're no longer accurate, I'd suggest updating them rather than removing them.
Please make sure to update function documentation for any of the functions where the return value is changing - a couple of the functions always return CHANNEL_RC_OK, now, so that should be noted in the function documentation with an explanation as to why that is the case.

Finally, while I understand the overall goal of this pull request, and while I'm overall okay with it, I think it's worth noting that, in at least some cases, this will allow a connection to continue without clipboard support, and with no indication to the user as to why that's the case. I think it's probably okay because that's how it currently works under FreeRDP 2.x - it silently ignores clipboard errors that FreeRDP 3.x now handles more properly; however, it seems like one of those situations that could be really frustrating from a user experience point of view. Instead of the user connection getting shut down and having to go find out from the admin why it isn't working, the connection will continue but every time the user tries to copy/paste something, it will silently and inexplicably fail. I don't know if there's really anything to do about that, but figure it's worth noting.

necouchman · 2026-05-16T12:53:11Z

+        guac_client_log(clipboard->client, GUAC_LOG_WARNING, "CLIPRDR: "
+                "Failed to send capabilities (error %d).", status);


I understand the goal of this - to continue the connection even if clipboard redirection fails to sync up capabilities with the server. That said, shouldn't we at least still return CHANNEL_RC_OK, here, and not try to send a format list if we've failed to send capabilities?

necouchman · 2026-05-16T12:57:32Z

-    /* FreeRDP-specific handlers for CLIPRDR are not assigned, and thus not
-     * callable, until after the relevant guac_rdp_clipboard structure is
-     * allocated and associated with the CliprdrClientContext */


Is this comment incorrect?

necouchman · 2026-05-16T13:06:11Z

        .msgFlags = CB_RESPONSE_OK
 #endif
    };
-    /* Report successful processing of format list */


Is this comment not accurate?

necouchman · 2026-05-16T13:10:05Z

+    int output_buf_size = clipboard->clipboard->available;
    const char* input = clipboard->clipboard->buffer;
-    char* output = guac_mem_alloc(GUAC_COMMON_CLIPBOARD_MAX_LENGTH);
+    char* output = guac_mem_alloc(output_buf_size);


I'm not sure if this overlaps with #659, or is related to the discussion on that thread regarding allocating available * 2 or available * 4? If this is an overlap, and is already covered by the changes in that change request, then it should probably not be, here.

necouchman · 2026-05-16T13:11:10Z

-    /* Send received clipboard data to the RDP server in the format
-     * requested */


Why is this comment being removed?

necouchman · 2026-05-16T13:13:01Z


    guac_mem_free(start);
-    return result;
+    return CHANNEL_RC_OK;


The documentation for this function should be updated to indicate either that it always returns CHANNEL_RC_OK despite failures, or the situations where failures are returned vs. CHANNEL_RC_OK.

necouchman · 2026-05-16T13:13:25Z

-    /* FreeRDP-specific handlers for CLIPRDR are not assigned, and thus not
-     * callable, until after the relevant guac_rdp_clipboard structure is
-     * allocated and associated with the CliprdrClientContext */


Why is this comment being removed?

necouchman · 2026-05-16T13:15:48Z


 void guac_rdp_clipboard_free(guac_rdp_clipboard* clipboard) {

-    /* Do nothing if the clipboard is not actually allocated */


Why has this comment been removed?

necouchman · 2026-05-16T13:15:59Z

    if (clipboard == NULL)
        return;

-    /* Free clipboard and underlying storage */


Why has this comment been removed?

escra changed the title ~~Harden CLIPRDR channel against FreeRDP 3.x clipboard state machine.~~ GUACAMOLE-2263: Harden CLIPRDR channel against FreeRDP 3.x clipboard state machine. Apr 14, 2026

necouchman requested changes May 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GUACAMOLE-2263: Harden CLIPRDR channel against FreeRDP 3.x clipboard state machine.#661

GUACAMOLE-2263: Harden CLIPRDR channel against FreeRDP 3.x clipboard state machine.#661
escra wants to merge 1 commit into
apache:staging/1.6.1from
ESCRA-GmbH:fix/cliprdr-freerdp3-hardening

escra commented Apr 14, 2026

Uh oh!

necouchman left a comment

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

necouchman May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		guac_client_log(clipboard->client, GUAC_LOG_WARNING, "CLIPRDR: "
		"Failed to send capabilities (error %d).", status);

		/* Send received clipboard data to the RDP server in the format
		* requested */


		void guac_rdp_clipboard_free(guac_rdp_clipboard* clipboard) {

		/* Do nothing if the clipboard is not actually allocated */

Conversation

escra commented Apr 14, 2026

Summary

Related

Affected versions

Test plan

Uh oh!

necouchman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants