From 44b04cafef56c5b5bfe731ff9a53bcf12b84bd22 Mon Sep 17 00:00:00 2001 From: ilammy Date: Sun, 9 Apr 2017 02:29:52 +0300 Subject: [PATCH] wClipboard: disallow Windows reserved names Another issue revealed during testing is that older Windows systems cannot handle the reserved file names well. While Windows 8 and 10 are fine (they silently abort the file transfer), using reserved names with Windows 7 can flat out crash explorer.exe or result into weird error messages like "fatal error: 0x00000000 ERROR_SUCCESS". This is not required by MS-RDPECLIP specification, but we should try to avoid this issue as not using reserved file names seems to be assumed a common sense in Windows protocols. The most convenient way to handle the issue would be on wClipboard level so that WinPR's clients do not bother with it. We should prohibit the reserved names from being used in FILEDESCRIPTOR, failing the conversion if we see such a file. POSIX subsystem (the only one at the moment) handles remote file names in two places so move the Unicode conversion and the new validation check into a separate function. The reserved file name predicate is placed into so that it can be used in other places too. For example, other wClipboard local file subsystems will need it. (It would be really nice to enforce this check somewhere in the common code, so that the subsystems can't miss it, but other places can miss some errors thus we're doing it here, as early as possible.) The predicate acts on separate file name components rather than full file names because the backslash is a reserved character too. If we process full file names this can result in phantom directory entry in the remote file name. Not to say that handling ready-made components spares us from splitting the full file name to extract them :) The implementation is... a bit verbose, but that's fine by me. In the absence of functions for case-insensitive wide string comparison and the need to check for the [0-9] at the end of some file names this is quite readable. Thanks to FAT and NTFS for being case-insensitive and to MS-DOS for having reserved file names in the first place. --- winpr/include/winpr/file.h | 2 + winpr/libwinpr/clipboard/posix.c | 68 ++++++++++++++++++--------- winpr/libwinpr/file/file.c | 80 ++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 21 deletions(-) diff --git a/winpr/include/winpr/file.h b/winpr/include/winpr/file.h index f2610e787..ed50cbed5 100644 --- a/winpr/include/winpr/file.h +++ b/winpr/include/winpr/file.h @@ -410,6 +410,8 @@ typedef struct _HANDLE_CREATOR pcCreateFileA CreateFileA; } HANDLE_CREATOR, *PHANDLE_CREATOR, *LPHANDLE_CREATOR; +WINPR_API BOOL ValidFileNameComponent(LPCWSTR lpFileName); + #endif /* _WIN32 */ #ifdef _UWP diff --git a/winpr/libwinpr/clipboard/posix.c b/winpr/libwinpr/clipboard/posix.c index 5e2581f03..aded955f7 100644 --- a/winpr/libwinpr/clipboard/posix.c +++ b/winpr/libwinpr/clipboard/posix.c @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -186,6 +187,45 @@ error: return NULL; } +/* + * Note that the function converts a single file name component, + * it does not take care of component separators. + */ +static WCHAR* convert_local_name_component_to_remote(const char* local_name) +{ + WCHAR* remote_name = NULL; + + /* + * Note that local file names are not actually guaranteed to be + * encoded in UTF-8. Filesystems and users can use whatever they + * want. The OS does not care, aside from special treatment of + * '\0' and '/' bytes. But we need to make some decision here. + * Assuming UTF-8 is currently the most sane thing. + */ + if (!ConvertToUnicode(CP_UTF8, 0, local_name, -1, &remote_name, 0)) + { + WLog_ERR(TAG, "Unicode conversion failed for %s", local_name); + goto error; + } + + /* + * Some file names are not valid on Windows. Check for these now + * so that we won't get ourselves into a trouble later as such names + * are known to crash some Windows shells when pasted via clipboard. + */ + if (!ValidFileNameComponent(remote_name)) + { + WLog_ERR(TAG, "invalid file name component: %s", local_name); + goto error; + } + + return remote_name; + +error: + free(remote_name); + return NULL; +} + static char* concat_local_name(const char* dir, const char* file) { size_t len_dir = 0; @@ -240,16 +280,9 @@ static BOOL add_directory_entry_to_list(const char* local_dir_name, const WCHAR* if ((strcmp(entry->d_name, ".") == 0) || (strcmp(entry->d_name, "..") == 0)) return TRUE; - /* - * As noted in add_file_to_list(), it is not always correct to assume - * that that file names are encoded in UTF-8. However, this is the - * most sane thing to do at the moment. - */ - if (!ConvertToUnicode(CP_UTF8, 0, entry->d_name, -1, &remote_base_name, 0)) - { - WLog_ERR(TAG, "Unicode conversion failed for %s", entry->d_name); + remote_base_name = convert_local_name_component_to_remote(entry->d_name); + if (!remote_base_name) return FALSE; - } local_name = concat_local_name(local_dir_name, entry->d_name); remote_name = concat_remote_name(remote_dir_name, remote_base_name); @@ -389,22 +422,15 @@ static BOOL process_file_name(const char* local_name, wArrayList* files) * Start with the base name of the file. text/uri-list contains the * exact files selected by the user, and we want the remote files * to have names relative to that selection. - * - * Note that local file names are not actually guaranteed to be - * encoded in UTF-8. Filesystems and users can use whatever they - * want. The OS does not care, aside from special treatment of - * '\0' and '/' bytes. But we need to make some decision here. - * Assuming UTF-8 is currently the most sane thing. */ base_name = basename(local_name); - if (!ConvertToUnicode(CP_UTF8, 0, base_name, -1, &remote_name, 0)) - { - WLog_ERR(TAG, "Unicode conversion failed for %s", base_name); - goto out; - } + + remote_name = convert_local_name_component_to_remote(base_name); + if (!remote_name) + return FALSE; result = add_file_to_list(local_name, remote_name, files); -out: + free(remote_name); return result; diff --git a/winpr/libwinpr/file/file.c b/winpr/libwinpr/file/file.c index 24679c542..a187d74fc 100644 --- a/winpr/libwinpr/file/file.c +++ b/winpr/libwinpr/file/file.c @@ -814,6 +814,86 @@ BOOL GetDiskFreeSpaceW(LPCWSTR lpwRootPathName, LPDWORD lpSectorsPerCluster, return ret; } +/** + * Check if a file name component is valid. + * + * Some file names are not valid on Windows. See "Naming Files, Paths, and Namespaces": + * https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx + */ +BOOL ValidFileNameComponent(LPCWSTR lpFileName) +{ + LPCWSTR c = NULL; + + /* CON */ + if ((lpFileName[0] != L'\0' && (lpFileName[0] == L'C' || lpFileName[0] == L'c')) && + (lpFileName[1] != L'\0' && (lpFileName[1] == L'O' || lpFileName[1] == L'o')) && + (lpFileName[2] != L'\0' && (lpFileName[2] == L'N' || lpFileName[2] == L'n')) && + (lpFileName[3] == L'\0')) + { + return FALSE; + } + + /* PRN */ + if ((lpFileName[0] != L'\0' && (lpFileName[0] == L'P' || lpFileName[0] == L'p')) && + (lpFileName[1] != L'\0' && (lpFileName[1] == L'R' || lpFileName[1] == L'r')) && + (lpFileName[2] != L'\0' && (lpFileName[2] == L'N' || lpFileName[2] == L'n')) && + (lpFileName[3] == L'\0')) + { + return FALSE; + } + + /* AUX */ + if ((lpFileName[0] != L'\0' && (lpFileName[0] == L'A' || lpFileName[0] == L'a')) && + (lpFileName[1] != L'\0' && (lpFileName[1] == L'U' || lpFileName[1] == L'u')) && + (lpFileName[2] != L'\0' && (lpFileName[2] == L'X' || lpFileName[2] == L'x')) && + (lpFileName[3] == L'\0')) + { + return FALSE; + } + + /* NUL */ + if ((lpFileName[0] != L'\0' && (lpFileName[0] == L'N' || lpFileName[0] == L'n')) && + (lpFileName[1] != L'\0' && (lpFileName[1] == L'U' || lpFileName[1] == L'u')) && + (lpFileName[2] != L'\0' && (lpFileName[2] == L'L' || lpFileName[2] == L'l')) && + (lpFileName[3] == L'\0')) + { + return FALSE; + } + + /* LPT0-9 */ + if ((lpFileName[0] != L'\0' && (lpFileName[0] == L'L' || lpFileName[0] == L'l')) && + (lpFileName[1] != L'\0' && (lpFileName[1] == L'P' || lpFileName[1] == L'p')) && + (lpFileName[2] != L'\0' && (lpFileName[2] == L'T' || lpFileName[2] == L't')) && + (lpFileName[3] != L'\0' && (L'0' <= lpFileName[3] && lpFileName[3] <= L'9')) && + (lpFileName[4] == L'\0')) + { + return FALSE; + } + + /* COM0-9 */ + if ((lpFileName[0] != L'\0' && (lpFileName[0] == L'C' || lpFileName[0] == L'c')) && + (lpFileName[1] != L'\0' && (lpFileName[1] == L'O' || lpFileName[1] == L'o')) && + (lpFileName[2] != L'\0' && (lpFileName[2] == L'M' || lpFileName[2] == L'm')) && + (lpFileName[3] != L'\0' && (L'0' <= lpFileName[3] && lpFileName[3] <= L'9')) && + (lpFileName[4] == L'\0')) + { + return FALSE; + } + + /* Reserved characters */ + for (c = lpFileName; *c; c++) + { + if ((*c == L'<') || (*c == L'>') || (*c == L':') || + (*c == L'"') || (*c == L'/') || (*c == L'\\') || + (*c == L'|') || (*c == L'?') || (*c == L'*')) + { + return FALSE; + } + } + + return TRUE; +} + #endif /* _WIN32 */ #ifdef _UWP