Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When the client clipboard contains an image, ConvertSelection(target="STRING") requests hang forever #2763

Closed
catern opened this issue Aug 2, 2023 · 7 comments · Fixed by #2766
Labels
bug confirmed confirmed reproduction

Comments

@catern
Copy link

catern commented Aug 2, 2023

xrdp version

0.9.21

Detailed xrdp version, build options

xrdp 0.9.21
  A Remote Desktop Protocol Server.
  Copyright (C) 2004-2020 Jay Sorg, Neutrino Labs, and all contributors.
  See https://github.com/neutrinolabs/xrdp for more information.

  Configure options:
      --build=x86_64-redhat-linux-gnu
      --host=x86_64-redhat-linux-gnu
      --program-prefix=
      --disable-dependency-tracking
      --prefix=/usr
      --exec-prefix=/usr
      --bindir=/usr/bin
      --sbindir=/usr/sbin
      --sysconfdir=/etc
      --datadir=/usr/share
      --includedir=/usr/include
      --libdir=/usr/lib64
      --libexecdir=/usr/libexec
      --localstatedir=/var
      --sharedstatedir=/var/lib
      --mandir=/usr/share/man
      --infodir=/usr/share/info
      --enable-pixman
      --enable-painter
      --enable-vsock
      --with-socketdir=/run/xrdp
      --with-imlib2
      build_alias=x86_64-redhat-linux-gnu
      host_alias=x86_64-redhat-linux-gnu
      CFLAGS=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection 
      LDFLAGS=-Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld
      PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig

  Compiled with OpenSSL 1.1.1k  FIPS 25 Mar 2021

Operating system & version

Rocky Linux 8.8 (Green Obsidian)

Installation method

dnf / apt / zypper / pkg / etc

Which backend do you use?

xorgxrdp

What desktop environment do you use?

happens even in an empty X session

Environment xrdp running on

VM

What's your client?

happens with both xfreerdp on Linux and Microsoft client on Windows

Area(s) with issue?

Clipboard

Steps to reproduce

  1. Run an RDP client connecting to xrdp on some server.
  2. Copy an image on the client-side.
  3. Running the following command on the server-side (you may have to run it a few times, xrdp seems to do some caching):
    xclip -out -selection clipboard

✔️ Expected Behavior

xsel should return immediately with the output "Error: target STRING not available", as on normal X.

❌ Actual Behavior

xsel hangs indefinitely.

Anything else?

No response

@catern catern added the bug label Aug 2, 2023
@matt335672
Copy link
Member

Hi @catern

Thanks for raising this rather interesting fault.

I can reproduce it on 0.9.21 with a Windows 10 client and the attached test.png.

I'm using xclip exclusively here rather than xsel.

  1. Copy test.png to the Windows filesystem.
  2. Open test.png in Paint (not Paint 3D)
  3. Select and copy the whole image
  4. On the Linux desktop using xclip :-
$ xclip -o -selection clipboard -t TARGETS
TARGETS
TIMESTAMP
MULTIPLE
STRING
UTF8_STRING
image/bmp
$ xclip -o -selection clipboard -t STRING
<hangs>

Interestingly, the client seems to be advertising that a STRING conversion target is available, but something seems to be getting lost in the plumbing somewhere when it is requested. Are you seeing a similar result with -t TARGETS? If not, can you give me a procedure where this doesn't happen?

I'll try the latest development version next to make sure we haven't already fixed this.

@matt335672 matt335672 added the confirmed confirmed reproduction label Aug 2, 2023
@catern
Copy link
Author

catern commented Aug 2, 2023

Yes, I see the same output from xclip -o -selection clipboard -t TARGETS, except I don't have image/bmp. Possibly that's an artifact of the different reproduction approach I'm using.

(Btw, I'm able to reproduce the problem by just right clicking an image in Chrome and selecting "Copy image" - I'm normally on a Linux client so I don't have Paint easily available :) )

@matt335672
Copy link
Member

The problem exists on devel too.

I've traced it through here, and it seems caused by the chansrv clipboard code reporting that CF_TEXT or CF_UNICODETEXT is available from the RDP client side even when the RDP client hasn't said that text is available in its CLIPRDR_FORMAT_ANNOUNCE message. When an X11 client then requests one of these formats the RDP client responds to say that the format isn't available. This response isn't handled properly and the X11 client hangs.

In other words there are two problems here:-

  1. The clipboard code is announcing the availability of text even when there is none available.
  2. The clipboard code is not properly handing an error for unavailable data.

2 is the more important one, as this can happen normally if the clipboard contents are changed between the format announcement and the format request.

@metalefty
Copy link
Member

Thanks, Matt for your investigation. I also looked around the code, the same opinion as you.

@jsorg71
Copy link
Contributor

jsorg71 commented Aug 3, 2023

Yes you guys are right, chansrv should not report string and utfstring unless the client reports text and unicode. I have a patch for this around, I'll see if I can find it.

@jsorg71
Copy link
Contributor

jsorg71 commented Aug 3, 2023

Something like this, the fix is to not always report text unless client reports is can do text.

diff --git a/sesman/chansrv/clipboard.c b/sesman/chansrv/clipboard.c
index 80609bc5..34e53755 100644
--- a/sesman/chansrv/clipboard.c
+++ b/sesman/chansrv/clipboard.c
@@ -2669,9 +2669,17 @@ clipboard_event_selection_request(XEvent *xevent)
         atom_buf[0] = g_targets_atom;
         atom_buf[1] = g_timestamp_atom;
         atom_buf[2] = g_multiple_atom;
-        atom_buf[3] = XA_STRING;
-        atom_buf[4] = g_utf8_atom;
-        atom_count = 5;
+        atom_count = 3;
+        if (clipboard_find_format_id(CB_FORMAT_TEXT))
+        {
+            atom_buf[atom_count] = XA_STRING;
+            atom_count++;
+        }
+        if (clipboard_find_format_id(CB_FORMAT_UNICODETEXT))
+        {
+            atom_buf[atom_count] = g_utf8_atom;
+            atom_count++;
+        }
         if (clipboard_find_format_id(CB_FORMAT_DIB) >= 0)
         {
             log_debug("  reporting image/bmp");

@matt335672
Copy link
Member

Thanks for that Jay.

I suspect it's a bit more complicated. I see we also support text when FileGroupDescriptorW is set by the client:-

else if ((lxev->target == XA_STRING) || (lxev->target == g_utf8_atom))
{
if (clipboard_find_format_id(g_file_format_id) >= 0)
{
LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_event_selection_request: "
"text requested when files available");
if (g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_FILE)

I'll work on a PR to fix both issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed confirmed reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants