summaryrefslogtreecommitdiff
path: root/Fixed-path-validation-in-drive-channel.patch
diff options
context:
space:
mode:
Diffstat (limited to 'Fixed-path-validation-in-drive-channel.patch')
-rw-r--r--Fixed-path-validation-in-drive-channel.patch296
1 files changed, 296 insertions, 0 deletions
diff --git a/Fixed-path-validation-in-drive-channel.patch b/Fixed-path-validation-in-drive-channel.patch
new file mode 100644
index 0000000..e4567f2
--- /dev/null
+++ b/Fixed-path-validation-in-drive-channel.patch
@@ -0,0 +1,296 @@
+From 865ba07a0fd4fbc7a8203482411aacca3bbfbb9f Mon Sep 17 00:00:00 2001
+From: akallabeth <akallabeth@posteo.net>
+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(-)
+
+diff --git a/channels/drive/client/drive_file.c b/channels/drive/client/drive_file.c
+index 305438593..1ea4ab9da 100644
+--- a/channels/drive/client/drive_file.c
++++ b/channels/drive/client/drive_file.c
+@@ -34,6 +34,7 @@
+ #include <stdlib.h>
+ #include <string.h>
+ #include <time.h>
++#include <assert.h>
+
+ #include <winpr/wtypes.h>
+ #include <winpr/crt.h>
+@@ -61,10 +62,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 +79,82 @@ static void drive_file_fix_path(WCHAR* path)
+ #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 +162,11 @@ static BOOL drive_file_remove_dir(const WCHAR* path)
+ 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 +176,7 @@ static BOOL drive_file_remove_dir(const WCHAR* path)
+
+ 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 +184,7 @@ static BOOL drive_file_remove_dir(const WCHAR* path)
+ 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 +360,13 @@ static BOOL drive_file_init(DRIVE_FILE* file)
+ 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 +386,7 @@ DRIVE_FILE* drive_file_new(const WCHAR* base_path, const WCHAR* path, UINT32 Pat
+ 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 +741,10 @@ BOOL drive_file_set_information(DRIVE_FILE* file, UINT32 FsInformationClass, UIN
+ 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 +783,7 @@ BOOL drive_file_set_information(DRIVE_FILE* file, UINT32 FsInformationClass, UIN
+ }
+
+ 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 +797,7 @@ BOOL drive_file_query_directory(DRIVE_FILE* file, UINT32 FsInformationClass, BYT
+ 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);
+diff --git a/channels/drive/client/drive_file.h b/channels/drive/client/drive_file.h
+index ed789d6f0..6d3bd7045 100644
+--- a/channels/drive/client/drive_file.h
++++ b/channels/drive/client/drive_file.h
+@@ -51,9 +51,9 @@ struct _DRIVE_FILE
+ 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_query_information(DRIVE_FILE* file, UINT32 FsInformationClass, w
+ 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 */
+diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c
+index 1b5422522..d3776381c 100644
+--- a/channels/drive/client/drive_main.c
++++ b/channels/drive/client/drive_main.c
+@@ -184,8 +184,8 @@ static UINT drive_process_irp_create(DRIVE_DEVICE* drive, IRP* irp)
+
+ 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 @@ static UINT drive_process_irp_query_directory(DRIVE_DEVICE* drive, IRP* irp)
+ 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());
+ }
+--
+2.37.1
+
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-30 16:44:45 +0000