diff --git a/debian/changelog b/debian/changelog index e30d8e2..e5133ea 100644 --- a/debian/changelog +++ b/debian/changelog @@ -13,6 +13,7 @@ freerdp2 (2.3.0+dfsg1-2+deb10u4) UNRELEASED; urgency=medium - CVE-2022-39316 - Out of bound read in zgfx decoder and - CVE-2022-39318 - Division by zero in urbdrc channel - CVE-2022-39319 - Missing length validation in urbdrc channel + - CVE-2022-39347 - Missing path sanitation with `drive` channel -- Tobias Frost Sat, 28 Oct 2023 18:12:57 +0200 diff --git a/debian/patches/0056-CVE-2022-39347.patch b/debian/patches/0056-CVE-2022-39347.patch new file mode 100644 index 0000000..cf59146 --- /dev/null +++ b/debian/patches/0056-CVE-2022-39347.patch @@ -0,0 +1,329 @@ +Description: CVE-2022-39347 - Missing path sanitation with `drive` channel +Origin: https://github.com/FreeRDP/FreeRDP/commit/027424c2c6c0991cb9c22f9511478229c9b17e5d +Bug: https://github.com/FreeRDP/FreeRDP/security/advisories/GHSA-c5xq-8v35-pffg +Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1024511 +From 027424c2c6c0991cb9c22f9511478229c9b17e5d Mon Sep 17 00:00:00 2001 +From: akallabeth +Date: Mon, 24 Oct 2022 10:41:55 +0200 +Subject: [PATCH] Fixed path validation in drive channel + +Check that canonical path is a subpath of the shared directory + +(cherry picked from commit 844c94e6d0438fa7bd8ff8d5513c3f69c3018b85) +--- + channels/drive/client/drive_file.c | 106 ++++++++++++++++++----------- + channels/drive/client/drive_file.h | 8 +-- + channels/drive/client/drive_main.c | 8 +-- + 3 files changed, 73 insertions(+), 49 deletions(-) + +--- a/channels/drive/client/drive_file.c ++++ b/channels/drive/client/drive_file.c +@@ -45,6 +45,45 @@ + + #include "drive_file.h" + ++#include ++ ++ ++static int _wcsncmp(const WCHAR* string1, const WCHAR* string2, size_t count) ++{ ++ assert(string1); ++ assert(string2); ++ ++ for (size_t x = 0; x < count; x++) ++ { ++ const WCHAR a = string1[x]; ++ const WCHAR b = string2[x]; ++ ++ if (a != b) ++ return (int)a - b; ++ else if ((a == '\0') || (b == '\0')) ++ return (int)a - b; ++ } ++ return 0; ++} ++ ++static WCHAR* _wcsstr(const WCHAR* str, const WCHAR* strSearch) ++{ ++ assert(str); ++ assert(strSearch); ++ ++ if (strSearch[0] == '\0') ++ return (WCHAR*)str; ++ ++ const size_t searchLen = _wcslen(strSearch); ++ while (*str) ++ { ++ if (_wcsncmp(str, strSearch, searchLen) == 0) ++ return (WCHAR*)str; ++ str++; ++ } ++ return NULL; ++} ++ + #ifdef WITH_DEBUG_RDPDR + #define DEBUG_WSTR(msg, wstr) \ + do \ +@@ -61,10 +100,14 @@ + } while (0) + #endif + +-static void drive_file_fix_path(WCHAR* path) ++static BOOL drive_file_fix_path(WCHAR* path, size_t length) + { + size_t i; +- size_t length = _wcslen(path); ++ ++ if ((length == 0) || (length > UINT32_MAX)) ++ return FALSE; ++ ++ assert(path); + + for (i = 0; i < length; i++) + { +@@ -75,58 +118,82 @@ + #ifdef WIN32 + + if ((length == 3) && (path[1] == L':') && (path[2] == L'/')) +- return; ++ return FALSE; + + #else + + if ((length == 1) && (path[0] == L'/')) +- return; ++ return FALSE; + + #endif + + if ((length > 0) && (path[length - 1] == L'/')) + path[length - 1] = L'\0'; ++ ++ return TRUE; + } + + static WCHAR* drive_file_combine_fullpath(const WCHAR* base_path, const WCHAR* path, +- size_t PathLength) ++ size_t PathWCharLength) + { +- WCHAR* fullpath; +- size_t base_path_length; ++ BOOL ok = FALSE; ++ WCHAR* fullpath = NULL; ++ size_t length; + +- if (!base_path || (!path && (PathLength > 0))) +- return NULL; ++ if (!base_path || (!path && (PathWCharLength > 0))) ++ goto fail; + +- base_path_length = _wcslen(base_path) * 2; +- fullpath = (WCHAR*)calloc(1, base_path_length + PathLength + sizeof(WCHAR)); ++ const size_t base_path_length = _wcsnlen(base_path, MAX_PATH); ++ length = base_path_length + PathWCharLength + 1; ++ fullpath = (WCHAR*)calloc(length, sizeof(WCHAR)); + + if (!fullpath) ++ goto fail; ++ ++ CopyMemory(fullpath, base_path, base_path_length * sizeof(WCHAR)); ++ if (path) ++ CopyMemory(&fullpath[base_path_length], path, PathWCharLength * sizeof(WCHAR)); ++ ++ if (!drive_file_fix_path(fullpath, length)) ++ goto fail; ++ ++ /* Ensure the path does not contain sequences like '..' */ ++ const WCHAR dotdot[] = { '.', '.', '\0' }; ++ if (_wcsstr(&fullpath[base_path_length], dotdot)) + { +- WLog_ERR(TAG, "malloc failed!"); +- return NULL; ++ char abuffer[MAX_PATH] = { 0 }; ++ ConvertFromUnicode(CP_UTF8, 0, &fullpath[base_path_length], -1, (char**)&abuffer, ++ ARRAYSIZE(abuffer) - 1, NULL, NULL); ++ ++ WLog_WARN(TAG, "[rdpdr] received invalid file path '%s' from server, aborting!", ++ &abuffer[base_path_length]); ++ goto fail; + } + +- CopyMemory(fullpath, base_path, base_path_length); +- if (path) +- CopyMemory((char*)fullpath + base_path_length, path, PathLength); +- drive_file_fix_path(fullpath); ++ ok = TRUE; ++fail: ++ if (!ok) ++ { ++ free(fullpath); ++ fullpath = NULL; ++ } + return fullpath; + } + + static BOOL drive_file_remove_dir(const WCHAR* path) + { +- WIN32_FIND_DATAW findFileData; ++ WIN32_FIND_DATAW findFileData = { 0 }; + BOOL ret = TRUE; +- HANDLE dir; +- WCHAR* fullpath; +- WCHAR* path_slash; +- size_t base_path_length; ++ HANDLE dir = INVALID_HANDLE_VALUE; ++ WCHAR* fullpath = NULL; ++ WCHAR* path_slash = NULL; ++ size_t base_path_length = 0; + + if (!path) + return FALSE; + +- base_path_length = _wcslen(path) * 2; +- path_slash = (WCHAR*)calloc(1, base_path_length + sizeof(WCHAR) * 3); ++ base_path_length = _wcslen(path); ++ path_slash = (WCHAR*)calloc(base_path_length + 3, sizeof(WCHAR)); + + if (!path_slash) + { +@@ -134,12 +201,11 @@ + return FALSE; + } + +- CopyMemory(path_slash, path, base_path_length); +- path_slash[base_path_length / 2] = L'/'; +- path_slash[base_path_length / 2 + 1] = L'*'; ++ CopyMemory(path_slash, path, base_path_length * sizeof(WCHAR)); ++ path_slash[base_path_length] = L'/'; ++ path_slash[base_path_length + 1] = L'*'; + DEBUG_WSTR("Search in %s", path_slash); + dir = FindFirstFileW(path_slash, &findFileData); +- path_slash[base_path_length / 2 + 1] = 0; + + if (dir == INVALID_HANDLE_VALUE) + { +@@ -149,7 +215,7 @@ + + do + { +- size_t len = _wcslen(findFileData.cFileName); ++ const size_t len = _wcsnlen(findFileData.cFileName, ARRAYSIZE(findFileData.cFileName)); + + if ((len == 1 && findFileData.cFileName[0] == L'.') || + (len == 2 && findFileData.cFileName[0] == L'.' && findFileData.cFileName[1] == L'.')) +@@ -157,7 +223,7 @@ + continue; + } + +- fullpath = drive_file_combine_fullpath(path_slash, findFileData.cFileName, len * 2); ++ fullpath = drive_file_combine_fullpath(path_slash, findFileData.cFileName, len); + DEBUG_WSTR("Delete %s", fullpath); + + if (findFileData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) +@@ -333,13 +399,13 @@ + return file->file_handle != INVALID_HANDLE_VALUE; + } + +-DRIVE_FILE* drive_file_new(const WCHAR* base_path, const WCHAR* path, UINT32 PathLength, UINT32 id, +- UINT32 DesiredAccess, UINT32 CreateDisposition, UINT32 CreateOptions, +- UINT32 FileAttributes, UINT32 SharedAccess) ++DRIVE_FILE* drive_file_new(const WCHAR* base_path, const WCHAR* path, UINT32 PathWCharLength, ++ UINT32 id, UINT32 DesiredAccess, UINT32 CreateDisposition, ++ UINT32 CreateOptions, UINT32 FileAttributes, UINT32 SharedAccess) + { + DRIVE_FILE* file; + +- if (!base_path || (!path && (PathLength > 0))) ++ if (!base_path || (!path && (PathWCharLength > 0))) + return NULL; + + file = (DRIVE_FILE*)calloc(1, sizeof(DRIVE_FILE)); +@@ -359,7 +425,7 @@ + file->CreateDisposition = CreateDisposition; + file->CreateOptions = CreateOptions; + file->SharedAccess = SharedAccess; +- drive_file_set_fullpath(file, drive_file_combine_fullpath(base_path, path, PathLength)); ++ drive_file_set_fullpath(file, drive_file_combine_fullpath(base_path, path, PathWCharLength)); + + if (!drive_file_init(file)) + { +@@ -714,13 +780,10 @@ + return FALSE; + + fullpath = drive_file_combine_fullpath(file->basepath, (WCHAR*)Stream_Pointer(input), +- FileNameLength); ++ FileNameLength / sizeof(WCHAR)); + + if (!fullpath) +- { +- WLog_ERR(TAG, "drive_file_combine_fullpath failed!"); + return FALSE; +- } + + #ifdef _WIN32 + +@@ -759,7 +822,7 @@ + } + + BOOL drive_file_query_directory(DRIVE_FILE* file, UINT32 FsInformationClass, BYTE InitialQuery, +- const WCHAR* path, UINT32 PathLength, wStream* output) ++ const WCHAR* path, UINT32 PathWCharLength, wStream* output) + { + size_t length; + WCHAR* ent_path; +@@ -773,7 +836,7 @@ + if (file->find_handle != INVALID_HANDLE_VALUE) + FindClose(file->find_handle); + +- ent_path = drive_file_combine_fullpath(file->basepath, path, PathLength); ++ ent_path = drive_file_combine_fullpath(file->basepath, path, PathWCharLength); + /* open new search handle and retrieve the first entry */ + file->find_handle = FindFirstFileW(ent_path, &file->find_data); + free(ent_path); +--- a/channels/drive/client/drive_file.h ++++ b/channels/drive/client/drive_file.h +@@ -51,9 +51,9 @@ + UINT32 CreateOptions; + }; + +-DRIVE_FILE* drive_file_new(const WCHAR* base_path, const WCHAR* path, UINT32 PathLength, UINT32 id, +- UINT32 DesiredAccess, UINT32 CreateDisposition, UINT32 CreateOptions, +- UINT32 FileAttributes, UINT32 SharedAccess); ++DRIVE_FILE* drive_file_new(const WCHAR* base_path, const WCHAR* path, UINT32 PathWCharLength, ++ UINT32 id, UINT32 DesiredAccess, UINT32 CreateDisposition, ++ UINT32 CreateOptions, UINT32 FileAttributes, UINT32 SharedAccess); + BOOL drive_file_free(DRIVE_FILE* file); + + BOOL drive_file_open(DRIVE_FILE* file); +@@ -64,6 +64,6 @@ + BOOL drive_file_set_information(DRIVE_FILE* file, UINT32 FsInformationClass, UINT32 Length, + wStream* input); + BOOL drive_file_query_directory(DRIVE_FILE* file, UINT32 FsInformationClass, BYTE InitialQuery, +- const WCHAR* path, UINT32 PathLength, wStream* output); ++ const WCHAR* path, UINT32 PathWCharLength, wStream* output); + + #endif /* FREERDP_CHANNEL_DRIVE_FILE_H */ +--- a/channels/drive/client/drive_main.c ++++ b/channels/drive/client/drive_main.c +@@ -184,8 +184,8 @@ + + path = (const WCHAR*)Stream_Pointer(irp->input); + FileId = irp->devman->id_sequence++; +- file = drive_file_new(drive->path, path, PathLength, FileId, DesiredAccess, CreateDisposition, +- CreateOptions, FileAttributes, SharedAccess); ++ file = drive_file_new(drive->path, path, PathLength / sizeof(WCHAR), FileId, DesiredAccess, ++ CreateDisposition, CreateOptions, FileAttributes, SharedAccess); + + if (!file) + { +@@ -636,8 +636,8 @@ + irp->IoStatus = STATUS_UNSUCCESSFUL; + Stream_Write_UINT32(irp->output, 0); /* Length */ + } +- else if (!drive_file_query_directory(file, FsInformationClass, InitialQuery, path, PathLength, +- irp->output)) ++ else if (!drive_file_query_directory(file, FsInformationClass, InitialQuery, path, ++ PathLength / sizeof(WCHAR), irp->output)) + { + irp->IoStatus = drive_map_windows_err(GetLastError()); + } diff --git a/debian/patches/series b/debian/patches/series index 25dd8c6..4508feb 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -45,3 +45,4 @@ 0053-CVE-2022-39316.patch 0054-CVE-2022-39318.patch 0055-CVE-2022-39319.patch +0056-CVE-2022-39347.patch