File freerdp-CVE-2022-39347.patch of Package freerdp

Overview Repositories Revisions Requests Users Attributes Meta

File freerdp-CVE-2022-39347.patch of Package freerdp (Revision 3979d1be6a7ae3c50690615777528b21)

Currently displaying revision 3979d1be6a7ae3c50690615777528b21 , Show latest

From 2079e32d15536258fb6f15777d84c81711a8e250 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Mon, 24 Oct 2022 10:42:56 +0200
Subject: [PATCH 1/4] Added function _wcsncmp

* Compare WCHAR strings up to n characters

(cherry picked from commit 8178ed26a459356ece17414c6e871a7e0735a4ec)
---
 winpr/include/winpr/string.h |  2 ++
 winpr/libwinpr/crt/string.c  | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/winpr/include/winpr/string.h b/winpr/include/winpr/string.h
index 8ce83bc1d..3b907c444 100644
--- a/winpr/include/winpr/string.h
+++ b/winpr/include/winpr/string.h
@@ -57,6 +57,7 @@ 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);
@@ -70,6 +71,7 @@ extern "C"
 #else
 
 #define _wcscmp wcscmp
+#define _wcsncmp wcsncmp
 #define _wcslen wcslen
 #define _wcsnlen wcsnlen
 #define _wcschr wcschr
diff --git a/winpr/libwinpr/crt/string.c b/winpr/libwinpr/crt/string.c
index a25aac4bd..fc2a8fd41 100644
--- a/winpr/libwinpr/crt/string.c
+++ b/winpr/libwinpr/crt/string.c
@@ -97,7 +97,21 @@ int _wcscmp(const WCHAR* string1, const WCHAR* string2)
 
 	Data_Read_UINT16(string1, value1);
 	Data_Read_UINT16(string2, value2);
-	return value1 - value2;
+	return (int)value1 - value2;
+}
+
+int _wcsncmp(const WCHAR* string1, const WCHAR* string2, size_t count)
+{
+	size_t x;
+	for (x = 0; x < count; x++)
+	{
+		const WCHAR a = string1[x];
+		const WCHAR b = string2[x];
+
+		if (a != b)
+			return (int)a - b;
+	}
+	return 0;
 }
 
 /* _wcslen -> wcslen */
-- 
2.39.1

From 12d426e7a00dc00e957efb3ebbeca48c0b546ef9 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Thu, 10 Nov 2022 14:21:22 +0100
Subject: [PATCH 2/4] [winpr, crt] Fix wcs*cmp and wcs*len checks

(cherry picked from commit b60fac1a0470fe83e8d0b448f0fd7e9e6d6a0f96)
---
 winpr/libwinpr/crt/string.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/winpr/libwinpr/crt/string.c b/winpr/libwinpr/crt/string.c
index fc2a8fd41..45b301210 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 */
@@ -87,21 +88,28 @@ 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 (int)value1 - value2;
+	return 0;
 }
 
 int _wcsncmp(const WCHAR* string1, const WCHAR* string2, size_t count)
 {
+	WINPR_ASSERT(string1);
+	WINPR_ASSERT(string2);
+
 	size_t x;
 	for (x = 0; x < count; x++)
 	{
@@ -110,6 +118,8 @@ int _wcsncmp(const WCHAR* string1, const WCHAR* string2, size_t count)
 
 		if (a != b)
 			return (int)a - b;
+		else if ((a == '\0') || (b == '\0'))
+			return (int)a - b;
 	}
 	return 0;
 }
@@ -120,8 +130,7 @@ size_t _wcslen(const WCHAR* str)
 {
 	const WCHAR* p = (const WCHAR*)str;
 
-	if (!p)
-		return 0;
+	WINPR_ASSERT(p);
 
 	while (*p)
 		p++;
@@ -135,8 +144,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++)
 	{
-- 
2.39.1

From 943bcb0b4e16fbef29fd565f4d8ba7ec1d04b105 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Thu, 10 Nov 2022 15:54:28 +0100
Subject: [PATCH 3/4] [winpr, crt] Added wcsstr implementation

(cherry picked from commit 6c034ba6117a4efc9266e845fe9a9a92ed4ee61d)
---
 winpr/include/winpr/string.h |  3 +++
 winpr/libwinpr/crt/string.c  | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/winpr/include/winpr/string.h b/winpr/include/winpr/string.h
index 3b907c444..2d7126210 100644
--- a/winpr/include/winpr/string.h
+++ b/winpr/include/winpr/string.h
@@ -62,6 +62,8 @@ extern "C"
 	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);
 
@@ -74,6 +76,7 @@ extern "C"
 #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 45b301210..0cc19e4e0 100644
--- a/winpr/libwinpr/crt/string.c
+++ b/winpr/libwinpr/crt/string.c
@@ -155,6 +155,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)
-- 
2.39.1

From 1fd777814d384a240c4543d2efd5413a96a08389 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Mon, 24 Oct 2022 10:41:55 +0200
Subject: [PATCH 4/4] 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 | 107 ++++++++++++++++++-----------
 channels/drive/client/drive_file.h |   8 +--
 channels/drive/client/drive_main.c |   8 +--
 3 files changed, 74 insertions(+), 49 deletions(-)

diff --git a/channels/drive/client/drive_file.c b/channels/drive/client/drive_file.c
index 305438593..cf5785f9e 100644
--- a/channels/drive/client/drive_file.c
+++ b/channels/drive/client/drive_file.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <time.h>
 
+#include <winpr/assert.h>
 #include <winpr/wtypes.h>
 #include <winpr/crt.h>
 #include <winpr/path.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;
+
+	WINPR_ASSERT(path);
 
 	for (i = 0; i < length; i++)
 	{
@@ -75,58 +80,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 +163,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 +177,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 +185,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 +361,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 +387,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 +742,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 +784,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 +798,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 35dc704e0..b6cf2ad32 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)
 	{
@@ -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());
 	}
-- 
2.39.1

Places

File freerdp-CVE-2022-39347.patch of Package freerdp (Revision 3979d1be6a7ae3c50690615777528b21)

Places