CVE-2022-39347 安全更新: FreeRDP存在路径遍历漏洞,该漏洞源于缺少对“drive”通道的路径规范化和基本路径检查

This commit is contained in:
tansy 2023-03-10 11:02:10 +08:00 committed by cckylin-cibot
parent 51b1882b30
commit f74b631eba
3 changed files with 429 additions and 1 deletions

8
debian/changelog vendored
View File

@ -1,3 +1,9 @@
freerdp2 (2.8.1-ok6) yangtze; urgency=low
* Oceania-cube CVE-2022-39347 安全更新: FreeRDP存在路径遍历漏洞该漏洞源于缺少对“drive”通道的路径规范化和基本路径检查。攻击者利用该漏洞可以读取共享目录之外的文件。
-- tansy <Cream_milk@bupt.edu.cn> Fri, 10 Mar 2023 11:01:17 +0800
freerdp2 (2.8.1-ok5) yangtze; urgency=low
* mcdonaldsburger CVE-2022-39320 安全更新: FreeRDP可能会在太窄的类型上尝试整数加法这会导致缓冲区的分配太小无法容纳写入的数据
@ -26,4 +32,4 @@ freerdp2 (2.8.1-ok1) yangtze; urgency=medium
* Build for openKylin.
-- zhouganqing <zhouganqing@kylinos.cn> Mon, 21 Nov 2022 14:41:39 +0800
-- zhouganqing <zhouganqing@kylinos.cn> Mon, 21 Nov 2022 14:41:39 +0800

View File

@ -0,0 +1,421 @@
From: tansy <Cream_milk@bupt.edu.cn>
Date: Fri, 10 Mar 2023 11:02:10 +0800
Subject: =?utf-8?b?Q1ZFLTIwMjItMzkzNDcg5a6J5YWo5pu05pawOiBGcmVlUkRQ5a2Y5Zyo?=
=?utf-8?b?6Lev5b6E6YGN5Y6G5ryP5rSe77yM6K+l5ryP5rSe5rqQ5LqO57y65bCR5a+54oCc?=
=?utf-8?b?ZHJpdmXigJ3pgJrpgZPnmoTot6/lvoTop4TojIPljJblkozln7rmnKzot6/lvoQ=?=
=?utf-8?b?5qOA5p+l?=
---
channels/drive/client/drive_file.c | 106 +++++++++++++++++++++++--------------
channels/drive/client/drive_file.h | 8 +--
channels/drive/client/drive_main.c | 8 +--
winpr/include/winpr/string.h | 5 ++
winpr/libwinpr/crt/string.c | 63 ++++++++++++++++++----
5 files changed, 130 insertions(+), 60 deletions(-)
diff --git a/channels/drive/client/drive_file.c b/channels/drive/client/drive_file.c
index 3054385..1ea4ab9 100644
--- a/channels/drive/client/drive_file.c
+++ b/channels/drive/client/drive_file.c
@@ -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++)
{
@@ -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 ed789d6..6d3bd70 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 1b54225..d377638 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());
}
diff --git a/winpr/include/winpr/string.h b/winpr/include/winpr/string.h
index d7a849c..bb98dff 100644
--- a/winpr/include/winpr/string.h
+++ b/winpr/include/winpr/string.h
@@ -60,10 +60,13 @@ extern "C"
WINPR_API int _strnicmp(const char* string1, const char* string2, size_t count);
WINPR_API int _wcscmp(const WCHAR* string1, const WCHAR* string2);
+ WINPR_API int _wcsncmp(const WCHAR* string1, const WCHAR* string2, size_t count);
WINPR_API size_t _wcslen(const WCHAR* str);
WINPR_API size_t _wcsnlen(const WCHAR* str, size_t maxNumberOfElements);
+ WINPR_API WCHAR* _wcsstr(const WCHAR* str, const WCHAR* strSearch);
+
WINPR_API WCHAR* _wcschr(const WCHAR* str, WCHAR c);
WINPR_API WCHAR* _wcsrchr(const WCHAR* str, WCHAR c);
@@ -73,8 +76,10 @@ extern "C"
#else
#define _wcscmp wcscmp
+#define _wcsncmp wcsncmp
#define _wcslen wcslen
#define _wcsnlen wcsnlen
+#define _wcsstr wcsstr
#define _wcschr wcschr
#define _wcsrchr wcsrchr
diff --git a/winpr/libwinpr/crt/string.c b/winpr/libwinpr/crt/string.c
index 748210e..6596b42 100644
--- a/winpr/libwinpr/crt/string.c
+++ b/winpr/libwinpr/crt/string.c
@@ -26,6 +26,7 @@
#include <wctype.h>
#include <winpr/crt.h>
+#include <winpr/assert.h>
#include <winpr/endian.h>
/* String Manipulation (CRT): http://msdn.microsoft.com/en-us/library/f0151s4x.aspx */
@@ -97,17 +98,39 @@ int _strnicmp(const char* string1, const char* string2, size_t count)
int _wcscmp(const WCHAR* string1, const WCHAR* string2)
{
- WCHAR value1, value2;
+ WINPR_ASSERT(string1);
+ WINPR_ASSERT(string2);
- while (*string1 && (*string1 == *string2))
+ while (TRUE)
{
- string1++;
- string2++;
+ const WCHAR w1 = *string1++;
+ const WCHAR w2 = *string2++;
+
+ if (w1 != w2)
+ return (int)w1 - w2;
+ else if ((w1 == '\0') || (w2 == '\0'))
+ return (int)w1 - w2;
}
- Data_Read_UINT16(string1, value1);
- Data_Read_UINT16(string2, value2);
- return value1 - value2;
+ return 0;
+}
+
+int _wcsncmp(const WCHAR* string1, const WCHAR* string2, size_t count)
+{
+ WINPR_ASSERT(string1);
+ WINPR_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;
}
/* _wcslen -> wcslen */
@@ -116,8 +139,7 @@ size_t _wcslen(const WCHAR* str)
{
const WCHAR* p = (const WCHAR*)str;
- if (!p)
- return 0;
+ WINPR_ASSERT(p);
while (*p)
p++;
@@ -131,8 +153,7 @@ size_t _wcsnlen(const WCHAR* str, size_t max)
{
size_t x;
- if (!str)
- return 0;
+ WINPR_ASSERT(str);
for (x = 0; x < max; x++)
{
@@ -143,6 +164,26 @@ size_t _wcsnlen(const WCHAR* str, size_t max)
return x;
}
+/* _wcsstr -> wcsstr */
+
+WCHAR* _wcsstr(const WCHAR* str, const WCHAR* strSearch)
+{
+ WINPR_ASSERT(str);
+ WINPR_ASSERT(strSearch);
+
+ if (strSearch[0] == '\0')
+ return str;
+
+ const size_t searchLen = _wcslen(strSearch);
+ while (*str)
+ {
+ if (_wcsncmp(str, strSearch, searchLen) == 0)
+ return str;
+ str++;
+ }
+ return NULL;
+}
+
/* _wcschr -> wcschr */
WCHAR* _wcschr(const WCHAR* str, WCHAR c)

View File

@ -2,3 +2,4 @@
0002-CVE-2022-39318-FreeRDP.patch
0003-CVE-2022-39319-FreeRDP-urbdrc.patch
0004-CVE-2022-39320-FreeRDP.patch
0005-CVE-2022-39347-FreeRDP-drive.patch