From 6f1b8f04c61d2db6e95cc1d45a826eeda9acfdce Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 21 Dec 2017 14:39:32 +0100 Subject: [PATCH 1/5] Fixed check for reserved com devices. --- winpr/libwinpr/comm/comm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/winpr/libwinpr/comm/comm.c b/winpr/libwinpr/comm/comm.c index 746ce7b0a..40e1da412 100644 --- a/winpr/libwinpr/comm/comm.c +++ b/winpr/libwinpr/comm/comm.c @@ -1065,7 +1065,7 @@ BOOL DefineCommDevice(/* DWORD dwFlags,*/ LPCTSTR lpDeviceName, if (_tcsncmp(lpDeviceName, _T("\\\\.\\"), 4) != 0) { - if (!_IsReservedCommDeviceName(lpDeviceName)) + if (_IsReservedCommDeviceName(lpDeviceName)) { SetLastError(ERROR_INVALID_DATA); goto error_handle; @@ -1470,9 +1470,7 @@ HANDLE CommCreateFileA(LPCSTR lpDeviceName, DWORD dwDesiredAccess, return (HANDLE)pComm; error_handle: - CloseHandle(pComm); - return INVALID_HANDLE_VALUE; } From 04708b37e11c0a9c7542d82ab40d3de0e2726e72 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 21 Dec 2017 14:39:43 +0100 Subject: [PATCH 2/5] Fixed serious issues with SAM file parser The parser ommitted various checks during file parsing. Invalid syntax did crash the whole thing. --- winpr/libwinpr/utils/sam.c | 76 ++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/winpr/libwinpr/utils/sam.c b/winpr/libwinpr/utils/sam.c index f9287e936..eb3d23ab8 100644 --- a/winpr/libwinpr/utils/sam.c +++ b/winpr/libwinpr/utils/sam.c @@ -89,6 +89,9 @@ static BOOL SamLookupStart(WINPR_SAM* sam) size_t readSize; INT64 fileSize; + if (!sam || !sam->fp) + return FALSE; + _fseeki64(sam->fp, 0, SEEK_END); fileSize = _ftelli64(sam->fp); _fseeki64(sam->fp, 0, SEEK_SET); @@ -119,7 +122,6 @@ static BOOL SamLookupStart(WINPR_SAM* sam) sam->buffer[fileSize] = '\n'; sam->buffer[fileSize + 1] = '\0'; sam->line = strtok(sam->buffer, "\n"); - return TRUE; } @@ -153,55 +155,74 @@ static void HexStrToBin(char* str, BYTE* bin, int length) } } -BOOL SamReadEntry(WINPR_SAM *sam, WINPR_SAM_ENTRY *entry) +BOOL SamReadEntry(WINPR_SAM* sam, WINPR_SAM_ENTRY* entry) { - char* p[7]; - int LmHashLength; - int NtHashLength; + char* p[5]; + size_t LmHashLength; + size_t NtHashLength; + size_t count = 0; + char* cur; + + if (!sam || !entry || !sam->line) + return FALSE; + + cur = sam->line; + + while ((cur = strchr(cur, ':')) != NULL) + { + count++; + cur++; + } + + if (count < 4) + return FALSE; p[0] = sam->line; p[1] = strchr(p[0], ':') + 1; p[2] = strchr(p[1], ':') + 1; p[3] = strchr(p[2], ':') + 1; p[4] = strchr(p[3], ':') + 1; - p[5] = strchr(p[4], ':') + 1; - p[6] = p[0] + strlen(p[0]); + LmHashLength = (p[3] - p[2] - 1); + NtHashLength = (p[4] - p[3] - 1); + + if ((LmHashLength != 0) && (LmHashLength != 32)) + return FALSE; + + if ((NtHashLength != 0) && (NtHashLength != 32)) + return FALSE; + entry->UserLength = (UINT32)(p[1] - p[0] - 1); entry->User = (LPSTR) malloc(entry->UserLength + 1); + if (!entry->User) return FALSE; + entry->User[entry->UserLength] = '\0'; entry->DomainLength = (UINT32)(p[2] - p[1] - 1); - LmHashLength = (int)(p[3] - p[2] - 1); - NtHashLength = (int)(p[4] - p[3] - 1); memcpy(entry->User, p[0], entry->UserLength); if (entry->DomainLength > 0) { entry->Domain = (LPSTR) malloc(entry->DomainLength + 1); + if (!entry->Domain) { free(entry->User); entry->User = NULL; return FALSE; } + memcpy(entry->Domain, p[1], entry->DomainLength); entry->Domain[entry->DomainLength] = '\0'; } else - { entry->Domain = NULL; - } if (LmHashLength == 32) - { HexStrToBin(p[2], (BYTE*) entry->LmHash, 16); - } if (NtHashLength == 32) - { HexStrToBin(p[3], (BYTE*) entry->NtHash, 16); - } return TRUE; } @@ -241,12 +262,12 @@ void SamResetEntry(WINPR_SAM_ENTRY* entry) ZeroMemory(entry->NtHash, sizeof(entry->NtHash)); } -WINPR_SAM_ENTRY* SamLookupUserA(WINPR_SAM* sam, LPSTR User, UINT32 UserLength, LPSTR Domain, UINT32 DomainLength) +WINPR_SAM_ENTRY* SamLookupUserA(WINPR_SAM* sam, LPSTR User, UINT32 UserLength, LPSTR Domain, + UINT32 DomainLength) { - int length; + size_t length; BOOL found = FALSE; WINPR_SAM_ENTRY* entry; - entry = (WINPR_SAM_ENTRY*) calloc(1, sizeof(WINPR_SAM_ENTRY)); if (!entry) @@ -260,7 +281,7 @@ WINPR_SAM_ENTRY* SamLookupUserA(WINPR_SAM* sam, LPSTR User, UINT32 UserLength, L while (sam->line != NULL) { - length = (int) strlen(sam->line); + length = strlen(sam->line); if (length > 1) { @@ -278,6 +299,7 @@ WINPR_SAM_ENTRY* SamLookupUserA(WINPR_SAM* sam, LPSTR User, UINT32 UserLength, L } } } + SamResetEntry(entry); sam->line = strtok(NULL, "\n"); } @@ -294,9 +316,10 @@ out_fail: return entry; } -WINPR_SAM_ENTRY* SamLookupUserW(WINPR_SAM* sam, LPWSTR User, UINT32 UserLength, LPWSTR Domain, UINT32 DomainLength) +WINPR_SAM_ENTRY* SamLookupUserW(WINPR_SAM* sam, LPWSTR User, UINT32 UserLength, LPWSTR Domain, + UINT32 DomainLength) { - int length; + size_t length; BOOL Found = FALSE; BOOL UserMatch; BOOL DomainMatch; @@ -317,7 +340,7 @@ WINPR_SAM_ENTRY* SamLookupUserW(WINPR_SAM* sam, LPWSTR User, UINT32 UserLength, while (sam->line != NULL) { - length = (int) strlen(sam->line); + length = strlen(sam->line); if (length > 1) { @@ -325,6 +348,7 @@ WINPR_SAM_ENTRY* SamLookupUserW(WINPR_SAM* sam, LPWSTR User, UINT32 UserLength, { DomainMatch = 0; UserMatch = 0; + if (!SamReadEntry(sam, entry)) goto out_fail; @@ -334,10 +358,12 @@ WINPR_SAM_ENTRY* SamLookupUserW(WINPR_SAM* sam, LPWSTR User, UINT32 UserLength, { EntryDomainLength = (UINT32) strlen(entry->Domain) * 2; EntryDomain = (LPWSTR) malloc(EntryDomainLength + 2); + if (!EntryDomain) goto out_fail; + MultiByteToWideChar(CP_ACP, 0, entry->Domain, EntryDomainLength / 2, - (LPWSTR) EntryDomain, EntryDomainLength / 2); + (LPWSTR) EntryDomain, EntryDomainLength / 2); if (DomainLength == EntryDomainLength) { @@ -363,10 +389,12 @@ WINPR_SAM_ENTRY* SamLookupUserW(WINPR_SAM* sam, LPWSTR User, UINT32 UserLength, { EntryUserLength = (UINT32) strlen(entry->User) * 2; EntryUser = (LPWSTR) malloc(EntryUserLength + 2); + if (!EntryUser) goto out_fail; + MultiByteToWideChar(CP_ACP, 0, entry->User, EntryUserLength / 2, - (LPWSTR) EntryUser, EntryUserLength / 2); + (LPWSTR) EntryUser, EntryUserLength / 2); if (UserLength == EntryUserLength) { From d823586c3eace80e91555c2a33706f449f207c68 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 21 Dec 2017 15:16:24 +0100 Subject: [PATCH 3/5] Fixed parallel channel argument checks --- channels/parallel/client/parallel_main.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/channels/parallel/client/parallel_main.c b/channels/parallel/client/parallel_main.c index b8c818041..0b0b14685 100644 --- a/channels/parallel/client/parallel_main.c +++ b/channels/parallel/client/parallel_main.c @@ -375,8 +375,8 @@ static UINT parallel_free(DEVICE* device) UINT error; PARALLEL_DEVICE* parallel = (PARALLEL_DEVICE*) device; - if (MessageQueue_PostQuit(parallel->queue, 0) - && (WaitForSingleObject(parallel->thread, INFINITE) == WAIT_FAILED)) + if (!MessageQueue_PostQuit(parallel->queue, 0) + || (WaitForSingleObject(parallel->thread, INFINITE) == WAIT_FAILED)) { error = GetLastError(); WLog_ERR(TAG, "WaitForSingleObject failed with error %"PRIu32"!", error); @@ -405,7 +405,7 @@ UINT DeviceServiceEntry(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints) { char* name; char* path; - int i; + size_t i; size_t length; RDPDR_PARALLEL* device; PARALLEL_DEVICE* parallel; @@ -414,10 +414,10 @@ UINT DeviceServiceEntry(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints) name = device->Name; path = device->Path; - if (!name || (name[0] == '*')) + if (!name || (name[0] == '*') || !path) { /* TODO: implement auto detection of parallel ports */ - return CHANNEL_RC_OK; + return CHANNEL_RC_INITIALIZATION_ERROR; } if (name[0] && path[0]) From 4483751e08c98000d6e0698471d220484f3050ab Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 21 Dec 2017 15:19:29 +0100 Subject: [PATCH 4/5] Fixed help for /parallel --- client/common/cmdline.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/client/common/cmdline.c b/client/common/cmdline.c index 550f5e626..1f89660e6 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -307,7 +307,7 @@ BOOL freerdp_client_print_command_line_help_ex(int argc, char** argv, printf("Smartcard Redirection: /smartcard:\n"); printf("Serial Port Redirection: /serial:,,[SerCx2|SerCx|Serial],[permissive]\n"); printf("Serial Port Redirection: /serial:COM1,/dev/ttyS0\n"); - printf("Parallel Port Redirection: /parallel:\n"); + printf("Parallel Port Redirection: /parallel:,\n"); printf("Printer Redirection: /printer:,\n"); printf("\n"); printf("Audio Output Redirection: /sound:sys:oss,dev:1,format:1\n"); @@ -476,7 +476,6 @@ BOOL freerdp_client_add_device_channel(rdpSettings* settings, int count, settings->RedirectSmartCards = TRUE; settings->DeviceRedirection = TRUE; - smartcard = (RDPDR_SMARTCARD*) calloc(1, sizeof(RDPDR_SMARTCARD)); if (!smartcard) @@ -725,7 +724,8 @@ error_argv: return FALSE; } -static char** freerdp_command_line_parse_comma_separated_values_ex(const char* name, const char* list, +static char** freerdp_command_line_parse_comma_separated_values_ex(const char* name, + const char* list, size_t* count) { char** p; @@ -757,7 +757,8 @@ static char** freerdp_command_line_parse_comma_separated_values_ex(const char* n { const char* it = list; - while((it = strchr(it, ',')) != NULL) + + while ((it = strchr(it, ',')) != NULL) { it++; nCommas++; @@ -1760,6 +1761,7 @@ int freerdp_client_settings_parse_command_line_arguments(rdpSettings* settings, WLog_ERR(TAG, "Smart sizing and dynamic resolution are mutually exclusive options"); return COMMAND_LINE_ERROR_UNEXPECTED_VALUE; } + settings->SupportDisplayControl = TRUE; settings->DynamicResolutionUpdate = TRUE; } From 9804d5a4a700c2af9aa7a0f805faf053e0110934 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 8 Jan 2018 13:07:11 +0100 Subject: [PATCH 5/5] SamOpen return NULL if file was not opened. --- winpr/libwinpr/utils/sam.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/winpr/libwinpr/utils/sam.c b/winpr/libwinpr/utils/sam.c index eb3d23ab8..2065317db 100644 --- a/winpr/libwinpr/utils/sam.c +++ b/winpr/libwinpr/utils/sam.c @@ -79,6 +79,9 @@ WINPR_SAM* SamOpen(const char* filename, BOOL readOnly) else { WLog_DBG(TAG, "Could not open SAM file!"); + fclose(fp); + free(sam); + return NULL; } return sam;