neutrinolabs / xrdp

xrdp: an open source RDP server
http://www.xrdp.org/
Apache License 2.0
5.58k stars 1.73k forks source link

Specific filenames cause File transfer / drive redirection error #3165

Closed tsz8899 closed 1 month ago

tsz8899 commented 1 month ago

xrdp version

0.10.80

Detailed xrdp version, build options

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

  Configure options:
      --enable-fuse
      --enable-ipv6
      --enable-rdpsndaudin
      --enable-mp3lame
      LDFLAGS=-ldl

Operating system & version

debian deepin

Installation method

git clone & make install

Which backend do you use?

xorgxrdp

What desktop environment do you use?

xfce dde

Environment xrdp running on

No response

What's your client?

mstsc xfreerdp wfreerdp

Area(s) with issue?

Clipboard, File transfer / drive redirection

Steps to reproduce

Specific filenames cause File transfer / drive redirection error or crash

The anomalies are not due to specific single characters but are caused by combinations of multiple normal characters utf-8

1.When the filename contains specific characters, copying via the file clipboard fails. For example: "日照香炉生紫烟遥看瀑布挂前川飞流直下三千尺疑是银河落九天.txt" Pasting from local to xrdp session works normally. Copying within the xrdp session to thinclient_drives also works. But copying from the xrdp session and pasting to the local disk does not work.

2.thinclient_drives cannot see files with specific character names. For example: "暮从碧山下,山月随人归。却顾所来径,苍苍横翠微。相携及田家,童稚开荆扉。绿竹入幽径,青萝拂行衣。欢言得所憩,美酒聊共挥。长歌吟松风,曲尽河星稀。我醉君复乐,陶然共忘机。.txt" In the thinclient_drives directory of the xrdp session, other files can be seen, but this file is not visible. This phenomenon also occurs in file managers like thunar and dde-file-manager on Debian Deepin.

3.Copying files with specific character names causes thinclient_drives to crash. For example: "暮从碧山下山月随人归却顾所来径苍苍横翠微相携及田家童稚开荆扉绿竹入幽径青萝拂行衣欢言得所憩美酒聊共挥长歌吟松风曲尽河星稀我醉君复乐陶然共忘机.txt" Pasting from local to xrdp session works normally. But pressing CTRL+C on this file within the xrdp session causes thinclient_drives to crash instantly. This phenomenon also occurs in file managers like thunar and dde-file-manager on Debian Deepin.

4.Deepin dde-file-manager file clipboard behaves abnormally (complex situations driving me crash). all filenames have issues.possible that the dde-file-manager has a significant impact. Normal: Copying single or multiple files in the xrdp session's Thunar or dde-file-manager and pasting to the windows local disk works normally (client programs: mstsc, wfreerdp). Pasting into the xrdp session's Thunar File Manager works normally (client programs: mstsc, xfreerdp, wfreerdp). Copying single or multiple files locally, and seeing them in the thinclient_drives/.clipboard directory in the dde-file-manager (client programs: mstsc, xfreerdp, wfreerdp).

Abnormal: Copying single or multiple files in the xrdp session's Thunar or dde-file-manager and pasting to the linux local disk can copy file,but local linux system file Manager prompt: "file Concatenation Error". XRDP Session Error Message:[clipboard_send_file_data(clipboard_file.c:511)] clipboard_send_file_data: read error, want 131072 got 0. Any file Error Message:want 131072 got 0(client programs: xfreerdp). Copying single or multiple files locally, and pasting in the dde-file-manager has no effect (client program: mstsc). Copying single or multiple files locally, and pasting in the dde-file-manager sometimes has no effect, but works after renaming the file (client program: xfreerdp). Pasting works the first time in the dde-file-manager, but fails after copying other files. Closing and reopening the dde-file-manager allows the first paste, but the issue recurs with subsequent copies (client program: wfreerdp).

Other situations: Files with overly long filenames in English characters cannot be copied via the file clipboard. For example: aaa...aaaa.txt (233-255 characters). Copying within the xrdp session fails: xrdp-chansrv[43485]: [ERROR] clipboard_get_file: Can't add client-side file /home/temp/thinclient_drives/_home_bb/Desktop/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr to clipboard but pasting the same file from local to the xrdp session works. This might be due to the path+filename length exceeding 255 characters? The OS's maximum filename length may also affect this.

✔️ Expected Behavior

1.Filenames do not cause File transfer / drive redirection to crash.

2.dde-file-manager can use the file clipboard normally. provide help in analyzing the direction for solving these issues.

3.File transfer / drive redirection can handle special filenames.

Requested Feature: File transfer / drive redirection add the ability to copy directories.^^

❌ Actual Behavior

No response

Anything else?

No response

matt335672 commented 1 month ago

Thanks for the detailed report @tsz8899.

I'll have a look at this next week - it's a bit busy IRL at the moment.

tsz8899 commented 1 month ago

Thanks

new situation:

Normal: Copying single or multiple files in the xrdp session's Thunar or dde-file-manager and pasting to the windows local disk works normally (client programs: mstsc, wfreerdp). Abnormal: Copying single or multiple files in the xrdp session's Thunar or dde-file-manager and pasting to the linux local disk can copy file,but local linux system file Manager prompt: "file Concatenation Error". XRDP Session Error Message:[clipboard_send_file_data(clipboard_file.c:511)] clipboard_send_file_data: read error, want 131072 got 0. Any file Error Message:want 131072 got 0(client programs: xfreerdp).

I have limited knowledge of English and programming and can only rely on AI for assistance.

Regarding this issue, AI made some modifications to clipboard_file.c. The local Linux "File Concatenation Error" and XRDP Session Error Message: "want 131072 got 0" have disappeared. Currently, copying files to Linux and Windows to be normal.

The modifications are as follows:

static int
clipboard_send_file_data(int streamId, int lindex,
                         int nPositionLow, int cbRequested)
{
    struct stream *s;
    int size;
    int rv;
    int fd;
    char full_fn[256];
    struct cb_file_info *cfi;

    if (g_files_list == 0)
    {
        LOG_DEVEL(LOG_LEVEL_ERROR, "clipboard_send_file_data: error g_files_list is nil");
        clipboard_send_filecontents_response_fail(streamId);
        return 1;
    }
    cfi = (struct cb_file_info *)list_get_item(g_files_list, lindex);
    if (cfi == 0)
    {
        LOG_DEVEL(LOG_LEVEL_ERROR, "clipboard_send_file_data: error cfi is nil");
        clipboard_send_filecontents_response_fail(streamId);
        return 1;
    }
    LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_send_file_data: streamId %d lindex %d "
              "nPositionLow %d cbRequested %d", streamId, lindex,
              nPositionLow, cbRequested);
    g_snprintf(full_fn, 255, "%s/%s", cfi->pathname, cfi->filename);
    fd = g_file_open_ro(full_fn);
    if (fd == -1)
    {
        LOG(LOG_LEVEL_ERROR, "clipboard_send_file_data: file open [%s] failed: %s",
            full_fn, g_get_strerror());
        clipboard_send_filecontents_response_fail(streamId);
        return 1;
    }
    if (g_file_seek(fd, nPositionLow) < 0)
    {
        LOG(LOG_LEVEL_ERROR, "clipboard_send_file_data: seek error in file [%s]: %s",
            full_fn, g_get_strerror());
        g_file_close(fd);
        clipboard_send_filecontents_response_fail(streamId);
        return 1;
    }
    make_stream(s);
    init_stream(s, cbRequested + 64);
    size = g_file_read(fd, s->data + 12, cbRequested);

    // Handle end of file (EOF) separately
    if (size == 0)
    {
        LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_send_file_data: reached end of file");
        free_stream(s);
        g_file_close(fd);

        // Notify the client that the transfer is complete
        make_stream(s);
        init_stream(s, 64);
        out_uint16_le(s, CB_FILECONTENTS_RESPONSE);
        out_uint16_le(s, CB_RESPONSE_OK);
        out_uint32_le(s, 4); // dataLen = 4
        out_uint32_le(s, streamId);
        s_mark_end(s);
        send_channel_data(g_cliprdr_chan_id, s->data, (int)(s->end - s->data));
        free_stream(s);

        return 0; // Successfully reached the end of file, not an error
    }

    // Error handling if reading failed
    if (size < 0)
    {
        LOG_DEVEL(LOG_LEVEL_ERROR, "clipboard_send_file_data: read error, want %d got %d",
                  cbRequested, size);
        free_stream(s);
        g_file_close(fd);
        clipboard_send_filecontents_response_fail(streamId);
        return 1;
    }
    out_uint16_le(s, CB_FILECONTENTS_RESPONSE); /* 9 */
    out_uint16_le(s, CB_RESPONSE_OK); /* 1 status */
    out_uint32_le(s, size + 4);
    out_uint32_le(s, streamId);
    s->p += size;
    out_uint32_le(s, 0);
    s_mark_end(s);
    size = (int)(s->end - s->data);
    rv = send_channel_data(g_cliprdr_chan_id, s->data, size);
    free_stream(s);
    g_file_close(fd);

    /* Log who transferred which file via clipboard for the purpose of audit */
    LOG(LOG_LEVEL_INFO, "S2C: Transferred a file: filename=%s, uid=%d", full_fn, g_getuid());

    return rv;
}

However, I am unsure if the AI's modifications might have caused any hidden issues. Please help when you are not busy I continue to try to locate other problem logs

tsz8899 commented 1 month ago

Issues 1 and 3 are not related to special characters, but due to the number of characters in the file name. Currently, the code supports file names with up to 27 Chinese characters. assuming 1 Chinese character consumes the space of 3 English characters?

[ERROR] [clipboard_get_file(clipboard_file.c:211)] clipboard_get_file: Can't add client-side file /home/temp/thinclient_drives/_home_bb/暮从碧山下山月随人归却顾所来径苍苍横翠微相携及田▒ to clipboard
[ERROR] [segfault_signal_handler(chansrv.c:1643)] segfault_signal_handler: entered.......
[INFO ] [child_signal_handler(chansrv.c:1627)] child_signal_handler:

After changing clipboard_file.c filename pathname to 1024, the code now supports up to 81 Chinese characters. Additionally, copying files with more than 81 characters no longer causes the thinclient_drives to crash.

However, it's unclear if this change affects other functionalities.

tsz8899 commented 1 month ago

Issues 2 are not related to special characters, but due to the number of characters in the file name. [WARN ] [devredir_proc_query_dir_response(devredir.c:1286)] Buffer was 1 bytes too small for filename change name to "暮从碧山下,山月随人归。却顾所来径,苍苍横翠微。相携及田家,童稚开荆扉。绿竹入幽径,青萝拂行衣。欢言得所憩,美酒聊共挥。长歌吟松风,曲尽河星稀。我醉君复乐,陶然共忘机。.tx" to be normal

if devredir.c

#define FILE_DIRECTORY_INFORMATION_SIZE(FileNameLength) \
    ((4 + 4 + 8 + 8 + 8 + 8 + 8 + 8 + 4 + 4) + FileNameLength)

    xstream_seek(s_in, 4);  /* Length */

    if (IoStatus == STATUS_SUCCESS)
    {
        /* process FILE_DIRECTORY_INFORMATION structures */
        while (1)
        {
            /* Can we read up to the filename? */
            if (!s_check_rem_and_log(s_in,
                                     FILE_DIRECTORY_INFORMATION_SIZE(0),
                                     "FILE_DIRECTORY_INFORMATION"))
            {
                return -1;
            }

            char  filename[512];

filename 256->512 [ERROR] [xfuse_devredir_cb_enum_dir_add_entry(chansrv_fuse.c:1047)] xfs_add_entry() failed

If the issue is solely due to the file name character limit, it can be controlled, so let's disregard issue 2.

Issue 4 likely stems from the dde-file-manager program. It is possible that this file manager has special handling for file clipboard operations.

tsz8899 commented 1 month ago

Issue 4 feedback help for the dfm developer:

It is possible that when writing to the target file list clipboard, the "x-special/gnome-copied-files" field is written. When reading this field, it blocks the copying of files to the thinclient_drives/.clipboard/ directory. This value is then written back to the "x-special/gnome-copied-files" field.

When dfm receives the clipboard signal for dataChanged, it reads the "x-special/gnome-copied-files" field, which, being blocking, causes some applications to fail to read it.

Some file managers read the "x-special/gnome-copied-files" field only after a paste operation, at which point the field can be read normally.

matt335672 commented 1 month ago

Here's some initial analysis from my side:-

Issue 1

Filename "日照香炉生紫烟遥看瀑布挂前川飞流直下三千尺疑是银河落九天.txt" maps to the following 88 bytes using a UTF-8 encoding:-

0000000 e6 97 a5 e7 85 a7 e9 a6 99 e7 82 89 e7 94 9f e7
0000016 b4 ab e7 83 9f e9 81 a5 e7 9c 8b e7 80 91 e5 b8
0000032 83 e6 8c 82 e5 89 8d e5 b7 9d e9 a3 9e e6 b5 81
0000048 e7 9b b4 e4 b8 8b e4 b8 89 e5 8d 83 e5 b0 ba e7
0000064 96 91 e6 98 af e9 93 b6 e6 b2 b3 e8 90 bd e4 b9
0000080 9d e5 a4 a9 2e 74 78 74
0000088

This should clearly work, and needs to be fixed.

Issue 2

Filename "暮从碧山下,山月随人归。却顾所来径,苍苍横翠微。相携及田家,童稚开荆扉。绿竹入幽径,青萝拂行衣。欢言得所憩,美酒聊共挥。长歌吟松风,曲尽河星稀。我醉君复乐,陶然共忘机。.txt" maps to the following 256 characters using a UTF-8 encoding:-

0000000 e6 9a ae e4 bb 8e e7 a2 a7 e5 b1 b1 e4 b8 8b ef
0000016 bc 8c e5 b1 b1 e6 9c 88 e9 9a 8f e4 ba ba e5 bd
0000032 92 e3 80 82 e5 8d b4 e9 a1 be e6 89 80 e6 9d a5
0000048 e5 be 84 ef bc 8c e8 8b 8d e8 8b 8d e6 a8 aa e7
0000064 bf a0 e5 be ae e3 80 82 e7 9b b8 e6 90 ba e5 8f
0000080 8a e7 94 b0 e5 ae b6 ef bc 8c e7 ab a5 e7 a8 9a
0000096 e5 bc 80 e8 8d 86 e6 89 89 e3 80 82 e7 bb bf e7
0000112 ab b9 e5 85 a5 e5 b9 bd e5 be 84 ef bc 8c e9 9d
0000128 92 e8 90 9d e6 8b 82 e8 a1 8c e8 a1 a3 e3 80 82
0000144 e6 ac a2 e8 a8 80 e5 be 97 e6 89 80 e6 86 a9 ef
0000160 bc 8c e7 be 8e e9 85 92 e8 81 8a e5 85 b1 e6 8c
0000176 a5 e3 80 82 e9 95 bf e6 ad 8c e5 90 9f e6 9d be
0000192 e9 a3 8e ef bc 8c e6 9b b2 e5 b0 bd e6 b2 b3 e6
0000208 98 9f e7 a8 80 e3 80 82 e6 88 91 e9 86 89 e5 90
0000224 9b e5 a4 8d e4 b9 90 ef bc 8c e9 99 b6 e7 84 b6
0000240 e5 85 b1 e5 bf 98 e6 9c ba e3 80 82 2e 74 78 74
0000256

It maps to the following 89 UTF-16 characters:-

0000000 66ae 4ece 78a7 5c71 4e0b ff0c 5c71 6708
0000016 968f 4eba 5f52 3002 5374 987e 6240 6765
0000032 5f84 ff0c 82cd 82cd 6a2a 7fe0 0020 5fae
0000048 3002 76f8 643a 53ca 7530 5bb6 ff0c 7ae5
0000064 7a1a 5f00 8346 6249 3002 7eff 7af9 5165
0000080 5e7d 5f84 ff0c 9752 841d 62c2 884c 8863
0000096 3002 6b22 8a00 5f97 6240 61a9 ff0c 7f8e
0000112 9152 804a 5171 6325 3002 957f 6b4c 541f
0000128 677e 98ce ff0c 66f2 5c3d 6cb3 661f 7a00
0000144 3002 6211 9189 541b 590d 4e50 ff0c 9676
0000160 7136 5171 5fd8 673a 3002 002e 0074 0078
0000176 0074
0000178

I can't create a file with this name on an EXT4 or XFS filesystem - see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits

This file CAN be created on an exFAT filesystem though, so I think XRDP should be able to handle it.

Issue 3

As you say, this is probably a length limitation.

Issue 4

I suggest we look at this when the other issues are sorted out.

matt335672 commented 1 month ago

I've added #3173 (currently in draft) which increases the space for filenames from 256 to 1024 characters.

Filenames are now (mostly) dynamically allocated, rather than using a fixed length. The limit of 1024 is not used for any long-term allocations, and so can be increased further if necessary without significantly changing the memory used by chansrv.

I think this may solve issues 1 to 3, but I need to do more testing myself before I release it.

tsz8899 commented 1 month ago

Thank

I have updated chansrv_fuse.c, chansrv_xfs.h, and chansrv_xfs.c The behavior of issue 3 has changed. Currently, on the xrdp server (Debian Deepin), ctrl+c a special file does not immediately cause the thinclient_drives to disconnect. However, when pasting this file (or read clipboard data,with the mouse right-click)on the client (mstsc, wfreerdp, xfreerdp), thinclient_drives disconnects instantly.

issue 1 in the xrdp server with a btrfs file system + Windows client(mstsc wfreerdp NTFS) and Linux client (xfreerdp btrfs), it is not working . xrdp info:[ERROR] clipboard_get_file: file [/home/h/Desktop/日照香炉生紫烟遥看瀑布挂前川飞流直下三千尺疑是银河落九天.txt/home/h/Desktop] does not exist xfreerdp info:[WARN][com.freerdp.client.x11.cliprdr] - [xf_cliprdr_server_format_data_response]: failed to get clipboard data in format x-special/gnome-copied-files [source format FileGroupDescriptorW]

issue 2 filenames can't be created on the debian Linux (btrfs). issue 2 filenames can be created on the client Windows(NTFS) and deepin Linux (btrfs).

have also modified clipboard_file.c, which has improved issues 1-3 in the above environments.

struct cb_file_info
{
    char pathname[1024];
    char filename[1024];
    int flags;
    int size;
    tui64 time;
};
...

static int
clipboard_check_file(char *filename)
{
    char lfilename[1024];
    char jchr[8];
    int lindex;
    int index;

    g_memset(lfilename, 0, 1024);
...

static int
clipboard_get_file(const char *file, int bytes)
{
    int sindex;
    int pindex;
    int flags;
    char full_fn[1024]; /* /etc/xrdp/xrdp.ini */
    char filename[1024]; /* xrdp.ini */
    char pathname[1024]; /* /etc/xrdp */
...

    g_memset(pathname, 0, 1024);
    g_memset(filename, 0, 1024);
...

static int
clipboard_get_files(const char *files, int bytes)
{
    int index;
    int file_index;
    char file[2048];
...

static int
clipboard_send_file_data(int streamId, int lindex,
                         int nPositionLow, int cbRequested)
{
    struct stream *s;
    int size;
    int rv;
    int fd;
    char full_fn[1024];
matt335672 commented 1 month ago

Thanks @tsz8899

I'll have a look at each of these individually using a memory analysis checker tool to find out where things are going wrong.

matt335672 commented 1 month ago

I've found the reason for 1) which is a hard-coded limit of 256 for filename lengths. This can be exceeded when each UTF-8 byte is represented in '%xx' notation.

Current version of the PR pastes something, but the name is truncated on the Windows side. I need to look into this.

matt335672 commented 1 month ago

Issue 1 is now fixed. I'm out of time today, but I'll look at 2 and 3 when I can.

tsz8899 commented 1 month ago

updated clipboard_file.c e64277f28 issue 1 test Debian Deepin normal

matt335672 commented 1 month ago

Issue 2 fixed.

I've found issue 3, but no time to fix it today.

matt335672 commented 1 month ago

3173 now fixes issues 1, 2, and 3.

@tsz8899 - when you have time, can you take a look? I'm going to do some more testing here relating to memory.

tsz8899 commented 1 month ago

updated -b chansrv_clip_fixes https://github.com/matt335672/xrdp xrdp server Debian12 and Deepin client wfreerdp xfreerdp mstsc

issue 1 test normal

issue 2 test normal thinclient_drives can now display long file names. Although they cannot be copied to unsupported file systems, this does not affect their usability.

issue 3 test normal Tried various filenames and did not find any that could make thinclient_drives crash.

tsz8899 commented 1 month ago

issue 5

Normal: Copying single or multiple files in the xrdp session's Thunar or dde-file-manager and pasting to the windows local disk works normally (client programs: mstsc, wfreerdp). Abnormal: linux client xfreerdp Copying single or multiple files in the xrdp session's Thunar or dde-file-manager and pasting to the linux local disk can copy file,but local linux system file Manager prompt: "file Concatenation Error". XRDP Session Error Message:[clipboard_send_file_data(clipboard_file.c:511)] clipboard_send_file_data: read error, want 131072 got 0. Any file Error Message:want 131072 got 0(client programs: xfreerdp).

0801

change clipboard_file.c if (size < 1) to if (size < 0)

copying files to Linux and Windows to be normal.

tsz8899 commented 1 month ago

Issue 4 feedback help for the dfm developer:

Added a dde-file-manager clipboard identification field to xrdp clipboard.c, new version of dde-file-manager seems to have implemented special handling,and the issue is resolved@@.

matt335672 commented 1 month ago

@tsz8899 - great work!

I've just reproduced issue 5 and validated your fix as the best solution. I've pushed it to #3173 which I've just rebased and retested.

I'm not fully understanding where we are with issue 4. Is more work required on xrdp?

tsz8899 commented 1 month ago

thanks issue 4 new dfm resolved. no more work required on xrdp