Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fixed path validation in drive channel
Check that canonical path is a subpath of the shared directory

(cherry picked from commit 844c94e)
  • Loading branch information
akallabeth committed Nov 14, 2022
1 parent 0a62aba commit 027424c
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 49 deletions.
106 changes: 65 additions & 41 deletions channels/drive/client/drive_file.c
Expand Up @@ -61,10 +61,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;

WINPR_ASSERT(path);

for (i = 0; i < length; i++)
{
Expand All @@ -75,71 +79,94 @@ 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)
{
WLog_ERR(TAG, "malloc failed!");
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)
{
Expand All @@ -149,15 +176,15 @@ 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'.'))
{
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)
Expand Down Expand Up @@ -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));
Expand All @@ -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))
{
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions channels/drive/client/drive_file.h
Expand Up @@ -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);
Expand All @@ -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 */
8 changes: 4 additions & 4 deletions channels/drive/client/drive_main.c
Expand Up @@ -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)
{
Expand Down Expand Up @@ -639,8 +639,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());
}
Expand Down

0 comments on commit 027424c

Please sign in to comment.