From 67817886ab0b7685b89685b083e8e3d7152a954e Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 04:33:29 +0200 Subject: [PATCH 01/28] [winpr,clipboard] fix integer narrow --- winpr/libwinpr/clipboard/synthetic.c | 56 +++++++++++----------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/winpr/libwinpr/clipboard/synthetic.c b/winpr/libwinpr/clipboard/synthetic.c index 8482c3411..f1406bc26 100644 --- a/winpr/libwinpr/clipboard/synthetic.c +++ b/winpr/libwinpr/clipboard/synthetic.c @@ -165,37 +165,31 @@ static void* clipboard_synthesize_cf_unicodetext(wClipboard* clipboard, UINT32 f static void* clipboard_synthesize_utf8_string(wClipboard* clipboard, UINT32 formatId, const void* data, UINT32* pSize) { - size_t size = 0; - char* pDstData = NULL; - if (formatId == CF_UNICODETEXT) { - pDstData = ConvertWCharNToUtf8Alloc(data, *pSize / sizeof(WCHAR), &size); + size_t size = 0; + char* pDstData = ConvertWCharNToUtf8Alloc(data, *pSize / sizeof(WCHAR), &size); if (!pDstData) return NULL; - size = ConvertLineEndingToLF(pDstData, size); - *pSize = (UINT32)size; + const size_t rc = ConvertLineEndingToLF(pDstData, size); + WINPR_ASSERT(rc <= UINT32_MAX); + *pSize = (UINT32)rc; return pDstData; } else if ((formatId == CF_TEXT) || (formatId == CF_OEMTEXT) || (formatId == ClipboardGetFormatId(clipboard, mime_text_plain))) { - int rc = 0; - size = *pSize; - pDstData = (char*)malloc(size); + const size_t size = *pSize; + char* pDstData = calloc(size + 1, sizeof(char)); if (!pDstData) return NULL; CopyMemory(pDstData, data, size); - rc = ConvertLineEndingToLF(pDstData, size); - if (rc < 0) - { - free(pDstData); - return NULL; - } + const size_t rc = ConvertLineEndingToLF(pDstData, size); + WINPR_ASSERT(rc <= UINT32_MAX); *pSize = (UINT32)rc; return pDstData; } @@ -588,49 +582,43 @@ fail: static void* clipboard_synthesize_text_html(wClipboard* clipboard, UINT32 formatId, const void* data, UINT32* pSize) { - long beg = 0; - long end = 0; - const char* str = NULL; - char* begStr = NULL; - char* endStr = NULL; - long DstSize = -1; - BYTE* pDstData = NULL; + char* pDstData = NULL; if (formatId == ClipboardGetFormatId(clipboard, "HTML Format")) { - INT64 SrcSize = 0; - str = (const char*)data; - SrcSize = (INT64)*pSize; - begStr = strstr(str, "StartHTML:"); - endStr = strstr(str, "EndHTML:"); + const char* str = (const char*)data; + const size_t SrcSize = (INT64)*pSize; + const char* begStr = strstr(str, "StartHTML:"); + const char* endStr = strstr(str, "EndHTML:"); if (!begStr || !endStr) return NULL; errno = 0; - beg = strtol(&begStr[10], NULL, 10); + const long beg = strtol(&begStr[10], NULL, 10); if (errno != 0) return NULL; - end = strtol(&endStr[8], NULL, 10); + const long end = strtol(&endStr[8], NULL, 10); if (beg < 0 || end < 0 || (beg > SrcSize) || (end > SrcSize) || (beg >= end) || (errno != 0)) return NULL; - DstSize = end - beg; - pDstData = (BYTE*)malloc((size_t)(SrcSize - beg + 1)); + const size_t DstSize = (size_t)(end - beg); + pDstData = calloc(DstSize + 1, sizeof(char)); if (!pDstData) return NULL; CopyMemory(pDstData, &str[beg], DstSize); - DstSize = ConvertLineEndingToLF((char*)pDstData, DstSize); - *pSize = (UINT32)DstSize; + const size_t rc = ConvertLineEndingToLF(pDstData, DstSize); + WINPR_ASSERT(rc <= UINT32_MAX); + *pSize = (UINT32)rc; } - return (void*)pDstData; + return pDstData; } BOOL ClipboardInitSynthesizers(wClipboard* clipboard) From 952076afb754723e43dd069ed3ce03f82f471415 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:37:57 +0200 Subject: [PATCH 02/28] [winpr,crypto] fix integer narrow --- winpr/libwinpr/crypto/cipher.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/winpr/libwinpr/crypto/cipher.c b/winpr/libwinpr/crypto/cipher.c index e6aed7c7c..d1e2fd298 100644 --- a/winpr/libwinpr/crypto/cipher.c +++ b/winpr/libwinpr/crypto/cipher.c @@ -629,7 +629,9 @@ int winpr_Cipher_BytesToKey(int cipher, WINPR_MD_TYPE md, const void* salt, cons const EVP_CIPHER* evp_cipher = NULL; evp_md = winpr_openssl_get_evp_md(md); evp_cipher = winpr_openssl_get_evp_cipher(cipher); - return EVP_BytesToKey(evp_cipher, evp_md, salt, data, datal, count, key, iv); + WINPR_ASSERT(datal <= INT_MAX); + WINPR_ASSERT(count <= INT_MAX); + return EVP_BytesToKey(evp_cipher, evp_md, salt, data, (int)datal, (int)count, key, iv); #elif defined(WITH_MBEDTLS) int rv = 0; BYTE md_buf[64]; From a5b80925e21573ac520afc362bf358cd0ee458a8 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:28:49 +0200 Subject: [PATCH 03/28] [winpr,interlocked] fix integer narrow --- winpr/libwinpr/interlocked/test/TestInterlockedAccess.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/winpr/libwinpr/interlocked/test/TestInterlockedAccess.c b/winpr/libwinpr/interlocked/test/TestInterlockedAccess.c index 3daeb8b5b..1d00d7a00 100644 --- a/winpr/libwinpr/interlocked/test/TestInterlockedAccess.c +++ b/winpr/libwinpr/interlocked/test/TestInterlockedAccess.c @@ -151,7 +151,7 @@ int TestInterlockedAccess(int argc, char* argv[]) *Destination64 = 0x66778899AABBCCDD; oldValue64 = - InterlockedCompareExchange64(Destination64, 0x8899AABBCCDDEEFF, 0x66778899AABBCCDD); + InterlockedCompareExchange64(Destination64, 0x0899AABBCCDDEEFFLL, 0x66778899AABBCCDD); if (oldValue64 != 0x66778899AABBCCDD) { @@ -161,10 +161,10 @@ int TestInterlockedAccess(int argc, char* argv[]) return -1; } - if ((*Destination64) != (LONGLONG)0x8899AABBCCDDEEFFLL) + if ((*Destination64) != 0x0899AABBCCDDEEFFLL) { printf("InterlockedCompareExchange failure: Actual: 0x%016" PRIX64 - ", Expected: 0x8899AABBCCDDEEFF\n", + ", Expected: 0x0899AABBCCDDEEFFLL\n", *Destination64); return -1; } @@ -173,7 +173,7 @@ int TestInterlockedAccess(int argc, char* argv[]) *Destination64 = 0x66778899AABBCCDDLL; - oldValue64 = InterlockedCompareExchange64(Destination64, 0x8899AABBCCDDEEFFLL, 12345); + oldValue64 = InterlockedCompareExchange64(Destination64, 0x0899AABBCCDDEEFFLL, 12345); if (oldValue64 != 0x66778899AABBCCDDLL) { From cb2cbdabfee0d9248a43004a8955ae97b167ceff Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 02:52:28 +0200 Subject: [PATCH 04/28] [winpr,ncrypt] fix integer narrow --- winpr/libwinpr/ncrypt/test/TestNCryptSmartcard.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/winpr/libwinpr/ncrypt/test/TestNCryptSmartcard.c b/winpr/libwinpr/ncrypt/test/TestNCryptSmartcard.c index 34560ba2f..aeeb4c2ca 100644 --- a/winpr/libwinpr/ncrypt/test/TestNCryptSmartcard.c +++ b/winpr/libwinpr/ncrypt/test/TestNCryptSmartcard.c @@ -29,24 +29,21 @@ static void crypto_print_name(const BYTE* b, DWORD sz) { - X509_NAME* name = NULL; - X509* x509 = NULL; - BIO* bio = NULL; - char* ret = NULL; - - bio = BIO_new_mem_buf(b, sz); + if (sz > INT32_MAX) + return; + BIO* bio = BIO_new_mem_buf(b, (int)sz); if (!bio) return; - x509 = d2i_X509_bio(bio, NULL); + X509* x509 = d2i_X509_bio(bio, NULL); if (!x509) goto bio_release; - name = X509_get_subject_name(x509); + X509_NAME* name = X509_get_subject_name(x509); if (!name) goto x509_release; - ret = calloc(1024, sizeof(char)); + char* ret = calloc(1024, sizeof(char)); if (!ret) goto bio_release; From a0dbb901df3e261458b3189499382b4324fac1f1 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:18:31 +0200 Subject: [PATCH 05/28] [winpr,shell] fix integer narrow --- winpr/libwinpr/shell/shell.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/winpr/libwinpr/shell/shell.c b/winpr/libwinpr/shell/shell.c index 8ff8d2f64..2ef907709 100644 --- a/winpr/libwinpr/shell/shell.c +++ b/winpr/libwinpr/shell/shell.c @@ -45,14 +45,9 @@ BOOL GetUserProfileDirectoryA(HANDLE hToken, LPSTR lpProfileDir, LPDWORD lpcchSize) { - char* buf = NULL; - int buflen = 0; - int status = 0; - DWORD cchDirSize = 0; - struct passwd pwd; + struct passwd pwd = { 0 }; struct passwd* pw = NULL; - WINPR_ACCESS_TOKEN* token = NULL; - token = (WINPR_ACCESS_TOKEN*)hToken; + WINPR_ACCESS_TOKEN* token = (WINPR_ACCESS_TOKEN*)hToken; if (!AccessTokenIsValid(hToken)) return FALSE; @@ -63,17 +58,17 @@ BOOL GetUserProfileDirectoryA(HANDLE hToken, LPSTR lpProfileDir, LPDWORD lpcchSi return FALSE; } - buflen = sysconf(_SC_GETPW_R_SIZE_MAX); + long buflen = sysconf(_SC_GETPW_R_SIZE_MAX); if (buflen == -1) buflen = 8196; - buf = (char*)malloc(buflen); + char* buf = (char*)malloc(buflen); if (!buf) return FALSE; - status = getpwnam_r(token->Username, &pwd, buf, buflen, &pw); + const int status = getpwnam_r(token->Username, &pwd, buf, buflen, &pw); if ((status != 0) || !pw) { @@ -82,7 +77,7 @@ BOOL GetUserProfileDirectoryA(HANDLE hToken, LPSTR lpProfileDir, LPDWORD lpcchSi return FALSE; } - cchDirSize = strlen(pw->pw_dir) + 1; + const size_t cchDirSize = strlen(pw->pw_dir) + 1; if (!lpProfileDir || (*lpcchSize < cchDirSize)) { From 22d1810e122c4d07312371c89ceff77c3487d40d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:44:38 +0200 Subject: [PATCH 06/28] [winpr,sspi] fix integer narrow --- winpr/libwinpr/sspi/sspi_winpr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winpr/libwinpr/sspi/sspi_winpr.c b/winpr/libwinpr/sspi/sspi_winpr.c index 3c82b0dab..ed17a9faf 100644 --- a/winpr/libwinpr/sspi/sspi_winpr.c +++ b/winpr/libwinpr/sspi/sspi_winpr.c @@ -1693,7 +1693,7 @@ static SECURITY_STATUS SEC_ENTRY winpr_DeleteSecurityContext(PCtxtHandle phConte return SEC_E_UNSUPPORTED_FUNCTION; } - const UINT32 status = table->DeleteSecurityContext(phContext); + const SECURITY_STATUS status = table->DeleteSecurityContext(phContext); if (IsSecurityStatusError(status)) { From 57f69ad77d4bb5cbc29ff8c27f02ae3c3201f850 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:05:21 +0200 Subject: [PATCH 07/28] [winpr,synch] fix integer narrow --- winpr/libwinpr/synch/test/TestSynchBarrier.c | 48 ++++++++++---------- winpr/libwinpr/synch/timer.c | 4 +- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/winpr/libwinpr/synch/test/TestSynchBarrier.c b/winpr/libwinpr/synch/test/TestSynchBarrier.c index bbbc99310..835ce7058 100644 --- a/winpr/libwinpr/synch/test/TestSynchBarrier.c +++ b/winpr/libwinpr/synch/test/TestSynchBarrier.c @@ -70,49 +70,47 @@ out: static BOOL TestSynchBarrierWithFlags(DWORD dwFlags, DWORD dwThreads, DWORD dwLoops) { - HANDLE* threads = NULL; - struct test_params p; + BOOL rc = FALSE; DWORD dwStatus = 0; - DWORD expectedTrueCount = 0; - DWORD expectedFalseCount = 0; - p.threadCount = 0; - p.trueCount = 0; - p.falseCount = 0; - p.loops = dwLoops; - p.flags = dwFlags; - expectedTrueCount = dwLoops; - expectedFalseCount = dwLoops * (dwThreads - 1); + + struct test_params p = { + .threadCount = 0, .trueCount = 0, .falseCount = 0, .loops = dwLoops, .flags = dwFlags + }; + DWORD expectedTrueCount = dwLoops; + DWORD expectedFalseCount = dwLoops * (dwThreads - 1); + printf("%s: >> Testing with flags 0x%08" PRIx32 ". Using %" PRIu32 " threads performing %" PRIu32 " loops\n", __func__, dwFlags, dwThreads, dwLoops); - if (!(threads = calloc(dwThreads, sizeof(HANDLE)))) + HANDLE* threads = (HANDLE*)calloc(dwThreads, sizeof(HANDLE)); + if (!threads) { printf("%s: error allocatin thread array memory\n", __func__); return FALSE; } - if (!InitializeSynchronizationBarrier(&gBarrier, dwThreads, -1)) + if (dwThreads > INT32_MAX) + goto fail; + + if (!InitializeSynchronizationBarrier(&gBarrier, (LONG)dwThreads, -1)) { printf("%s: InitializeSynchronizationBarrier failed. GetLastError() = 0x%08x", __func__, GetLastError()); - free(threads); - DeleteSynchronizationBarrier(&gBarrier); - return FALSE; + goto fail; } if (!(gStartEvent = CreateEvent(NULL, TRUE, FALSE, NULL))) { printf("%s: CreateEvent failed with error 0x%08x", __func__, GetLastError()); - free(threads); - DeleteSynchronizationBarrier(&gBarrier); - return FALSE; + goto fail; } DWORD i = 0; for (; i < dwThreads; i++) { - if (!(threads[i] = CreateThread(NULL, 0, test_synch_barrier_thread, &p, 0, NULL))) + threads[i] = CreateThread(NULL, 0, test_synch_barrier_thread, &p, 0, NULL); + if (!threads[i]) { printf("%s: CreateThread failed for thread #%" PRIu32 " with error 0x%08x\n", __func__, i, GetLastError()); @@ -149,8 +147,6 @@ static BOOL TestSynchBarrierWithFlags(DWORD dwFlags, DWORD dwThreads, DWORD dwLo } } - free(threads); - if (!CloseHandle(gStartEvent)) { printf("%s: CloseHandle(gStartEvent) failed with error = 0x%08x)\n", __func__, @@ -158,8 +154,6 @@ static BOOL TestSynchBarrierWithFlags(DWORD dwFlags, DWORD dwThreads, DWORD dwLo InterlockedIncrement(&gErrorCount); } - DeleteSynchronizationBarrier(&gBarrier); - if (p.threadCount != (INT64)dwThreads) InterlockedIncrement(&gErrorCount); @@ -177,13 +171,17 @@ static BOOL TestSynchBarrierWithFlags(DWORD dwFlags, DWORD dwThreads, DWORD dwLo printf("%s: false count: %" PRId32 " (expected %" PRIu32 ")\n", __func__, p.falseCount, expectedFalseCount); + rc = TRUE; +fail: + free((void*)threads); + DeleteSynchronizationBarrier(&gBarrier); if (gErrorCount > 0) { printf("%s: Error test failed with %" PRId32 " reported errors\n", __func__, gErrorCount); return FALSE; } - return TRUE; + return rc; } int TestSynchBarrier(int argc, char* argv[]) diff --git a/winpr/libwinpr/synch/timer.c b/winpr/libwinpr/synch/timer.c index 5c1564ef7..6b40de177 100644 --- a/winpr/libwinpr/synch/timer.c +++ b/winpr/libwinpr/synch/timer.c @@ -515,8 +515,8 @@ BOOL SetWaitableTimer(HANDLE hTimer, const LARGE_INTEGER* lpDueTime, LONG lPerio if (lPeriod > 0) { - timer->timeout.it_interval.tv_sec = (lPeriod / 1000ULL); /* seconds */ - timer->timeout.it_interval.tv_nsec = (1000000ULL * (lPeriod % 1000ULL)); /* nanoseconds */ + timer->timeout.it_interval.tv_sec = (lPeriod / 1000LL); /* seconds */ + timer->timeout.it_interval.tv_nsec = (1000000LL * (lPeriod % 1000LL)); /* nanoseconds */ } if (lpDueTime->QuadPart != 0) From 4a0f996d42575e55ddce39b8d07cb8a381ab721d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:14:32 +0200 Subject: [PATCH 08/28] [winpr,timezone] fix integer narrow --- winpr/libwinpr/timezone/timezone.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/winpr/libwinpr/timezone/timezone.c b/winpr/libwinpr/timezone/timezone.c index 7b8422bf6..05ab5446c 100644 --- a/winpr/libwinpr/timezone/timezone.c +++ b/winpr/libwinpr/timezone/timezone.c @@ -49,24 +49,29 @@ static char* winpr_read_unix_timezone_identifier_from_file(FILE* fp) { const INT CHUNK_SIZE = 32; - INT64 rc = 0; - INT64 read = 0; - INT64 length = CHUNK_SIZE; - char* tzid = NULL; + size_t rc = 0; + size_t read = 0; + size_t length = CHUNK_SIZE; - tzid = (char*)malloc((size_t)length); + char* tzid = malloc(length); if (!tzid) return NULL; do { - rc = fread(tzid + read, 1, length - read - 1, fp); + rc = fread(tzid + read, 1, length - read - 1UL, fp); if (rc > 0) read += rc; - if (read < (length - 1)) + if (read < (length - 1UL)) break; + if (read > length - 1UL) + { + free(tzid); + return NULL; + } + length += CHUNK_SIZE; char* tmp = (char*)realloc(tzid, length); if (!tmp) From 8a042b33d60dd223eb822abd3830420cbd04350d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 02:50:19 +0200 Subject: [PATCH 09/28] [winpr,utils] fix integer narrow --- winpr/libwinpr/utils/cmdline.c | 17 +++++++++++------ winpr/libwinpr/utils/test/TestArrayList.c | 10 ++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/winpr/libwinpr/utils/cmdline.c b/winpr/libwinpr/utils/cmdline.c index 64b223c89..8e1c1074d 100644 --- a/winpr/libwinpr/utils/cmdline.c +++ b/winpr/libwinpr/utils/cmdline.c @@ -59,13 +59,11 @@ int CommandLineParseArgumentsA(int argc, LPSTR* argv, COMMAND_LINE_ARGUMENT_A* o { int status = 0; int count = 0; - size_t length = 0; BOOL notescaped = FALSE; const char* sigil = NULL; size_t sigil_length = 0; char* keyword = NULL; - size_t keyword_length = 0; - SSIZE_T keyword_index = 0; + size_t keyword_index = 0; char* separator = NULL; char* value = NULL; int toggle = 0; @@ -85,6 +83,7 @@ int CommandLineParseArgumentsA(int argc, LPSTR* argv, COMMAND_LINE_ARGUMENT_A* o for (int i = 1; i < argc; i++) { + size_t keyword_length = 0; BOOL found = FALSE; BOOL escaped = TRUE; @@ -108,7 +107,7 @@ int CommandLineParseArgumentsA(int argc, LPSTR* argv, COMMAND_LINE_ARGUMENT_A* o } sigil = argv[i]; - length = strlen(argv[i]); + size_t length = strlen(argv[i]); if ((sigil[0] == '/') && (flags & COMMAND_LINE_SIGIL_SLASH)) { @@ -202,14 +201,20 @@ int CommandLineParseArgumentsA(int argc, LPSTR* argv, COMMAND_LINE_ARGUMENT_A* o } else { - keyword_length = (length - keyword_index); + if (length < keyword_index) + { + log_error(flags, "Failed at index %d [%s]: Argument required", i, argv[i]); + return COMMAND_LINE_ERROR; + } + + keyword_length = length - keyword_index; value = NULL; } if (!escaped) continue; - for (int j = 0; options[j].Name != NULL; j++) + for (size_t j = 0; options[j].Name != NULL; j++) { COMMAND_LINE_ARGUMENT_A* cur = &options[j]; BOOL match = FALSE; diff --git a/winpr/libwinpr/utils/test/TestArrayList.c b/winpr/libwinpr/utils/test/TestArrayList.c index b67faf23e..ad70f3243 100644 --- a/winpr/libwinpr/utils/test/TestArrayList.c +++ b/winpr/libwinpr/utils/test/TestArrayList.c @@ -5,16 +5,14 @@ int TestArrayList(int argc, char* argv[]) { - SSIZE_T count = 0; SSIZE_T rc = 0; size_t val = 0; - wArrayList* arrayList = NULL; const size_t elemsToInsert = 10; WINPR_UNUSED(argc); WINPR_UNUSED(argv); - arrayList = ArrayList_New(TRUE); + wArrayList* arrayList = ArrayList_New(TRUE); if (!arrayList) return -1; @@ -24,9 +22,9 @@ int TestArrayList(int argc, char* argv[]) return -1; } - count = ArrayList_Count(arrayList); + size_t count = ArrayList_Count(arrayList); - printf("ArrayList count: %d\n", count); + printf("ArrayList count: %" PRIuz "\n", count); SSIZE_T index = ArrayList_IndexOf(arrayList, (void*)(size_t)6, -1, -1); @@ -71,7 +69,7 @@ int TestArrayList(int argc, char* argv[]) return -1; count = ArrayList_Count(arrayList); - printf("ArrayList count: %d\n", count); + printf("ArrayList count: %" PRIuz "\n", count); if (count != 0) return -1; From 47835465827a2b8751f00b3c6b2c635243018b09 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:48:55 +0200 Subject: [PATCH 10/28] [winpr,wlog] fix integer narrow --- winpr/libwinpr/utils/wlog/wlog.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/winpr/libwinpr/utils/wlog/wlog.c b/winpr/libwinpr/utils/wlog/wlog.c index d28cb521b..4791c15c1 100644 --- a/winpr/libwinpr/utils/wlog/wlog.c +++ b/winpr/libwinpr/utils/wlog/wlog.c @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -755,7 +756,8 @@ LONG WLog_GetFilterLogLevel(wLog* log) if (_stricmp(filter->Names[j], "*") == 0) { match = TRUE; - log->FilterLevel = filter->Level; + assert(filter->Level <= INT32_MAX); + log->FilterLevel = (LONG)filter->Level; break; } @@ -766,7 +768,10 @@ LONG WLog_GetFilterLogLevel(wLog* log) { match = log->NameCount == filter->NameCount; if (match) - log->FilterLevel = filter->Level; + { + assert(filter->Level <= INT32_MAX); + log->FilterLevel = (LONG)filter->Level; + } break; } } From e13273d94df3c63967985789c0abbdd23f7cb914 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:03:52 +0200 Subject: [PATCH 11/28] [utils,test] fix integer narrow --- libfreerdp/utils/test/TestEncodedTypes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfreerdp/utils/test/TestEncodedTypes.c b/libfreerdp/utils/test/TestEncodedTypes.c index b059d3bfe..80eab107b 100644 --- a/libfreerdp/utils/test/TestEncodedTypes.c +++ b/libfreerdp/utils/test/TestEncodedTypes.c @@ -136,8 +136,8 @@ static BOOL test_float_read_write_equal(double value) return FALSE; } const double diff = fabs(value - rvalue); - const UINT64 expdiff = diff * pow(10, exp); - if (expdiff > 0) + const double expdiff = diff * pow(10, exp); + if (expdiff >= 1.0) { (void)fprintf(stderr, "[%s(%lf)] read invalid value %lf from stream\n", __func__, value, rvalue); From b5dbe5a167dac5d9793231726cd175acada836fe Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 02:47:26 +0200 Subject: [PATCH 12/28] [emu,scard] fix integer narrow --- libfreerdp/emu/scard/smartcard_virtual_gids.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libfreerdp/emu/scard/smartcard_virtual_gids.c b/libfreerdp/emu/scard/smartcard_virtual_gids.c index a516c3896..7504551af 100644 --- a/libfreerdp/emu/scard/smartcard_virtual_gids.c +++ b/libfreerdp/emu/scard/smartcard_virtual_gids.c @@ -502,7 +502,7 @@ handle_error: return FALSE; } -static int get_rsa_key_size(const rdpPrivateKey* privateKey) +static size_t get_rsa_key_size(const rdpPrivateKey* privateKey) { WINPR_ASSERT(privateKey); @@ -1523,8 +1523,8 @@ BOOL vgids_init(vgidsContext* ctx, const char* cert, const char* privateKey, con goto init_failed; /* write container map DO */ - const int size = get_rsa_key_size(ctx->privateKey); - if (size <= 0) + const size_t size = get_rsa_key_size(ctx->privateKey); + if ((size == 0) || (size > UINT16_MAX / 8)) goto init_failed; cmrec.wKeyExchangeKeySizeBits = (WORD)size * 8; From 10bb42e4d83286754f599f0bd1bc4ff5dfc356ca Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 05:15:15 +0200 Subject: [PATCH 13/28] [server,proxy] fix integer narrowing --- server/proxy/pf_channel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/proxy/pf_channel.c b/server/proxy/pf_channel.c index baec62ffe..bbbb5549b 100644 --- a/server/proxy/pf_channel.c +++ b/server/proxy/pf_channel.c @@ -94,8 +94,8 @@ PfChannelResult channelTracker_update(ChannelStateTracker* tracker, const BYTE* UINT32 flags, size_t totalSize) { PfChannelResult result = PF_CHANNEL_RESULT_ERROR; - BOOL firstPacket = (flags & CHANNEL_FLAG_FIRST); - BOOL lastPacket = (flags & CHANNEL_FLAG_LAST); + BOOL firstPacket = (flags & CHANNEL_FLAG_FIRST) != 0; + BOOL lastPacket = (flags & CHANNEL_FLAG_LAST) != 0; WINPR_ASSERT(tracker); From f55a1e2bc3ce62fce15638934c899cfbbf4171b4 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:09:12 +0200 Subject: [PATCH 14/28] [proxy,modules] fix integer narrow --- .../proxy/modules/dyn-channel-dump/dyn-channel-dump.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/server/proxy/modules/dyn-channel-dump/dyn-channel-dump.cpp b/server/proxy/modules/dyn-channel-dump/dyn-channel-dump.cpp index 85867982a..40fb12f00 100644 --- a/server/proxy/modules/dyn-channel-dump/dyn-channel-dump.cpp +++ b/server/proxy/modules/dyn-channel-dump/dyn-channel-dump.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -332,7 +333,13 @@ static BOOL dump_dyn_channel_intercept(proxyPlugin* plugin, proxyData* pdata, vo WLog_ERR(TAG, "Could not write to stream"); return FALSE; } - stream.write(buffer, Stream_Length(data->data)); + const auto s = Stream_Length(data->data); + if (s > std::numeric_limits::max()) + { + WLog_ERR(TAG, "Stream length %" PRIuz " exceeds std::streamsize::max", s); + return FALSE; + } + stream.write(buffer, static_cast(s)); if (stream.fail()) { WLog_ERR(TAG, "Could not write to stream"); From 997ff57301f4e1fcabf7ab80e785860ea8ee08f2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:16:34 +0200 Subject: [PATCH 15/28] [proxy,rdpdr] fix integer narrow --- server/proxy/channels/pf_channel_rdpdr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/proxy/channels/pf_channel_rdpdr.c b/server/proxy/channels/pf_channel_rdpdr.c index 2ab15fba9..18f5fdfb0 100644 --- a/server/proxy/channels/pf_channel_rdpdr.c +++ b/server/proxy/channels/pf_channel_rdpdr.c @@ -457,7 +457,7 @@ static UINT rdpdr_process_client_name_request(pf_channel_server_context* rdpdr, return ERROR_INVALID_DATA; Stream_Read_UINT32(s, unicodeFlag); - rdpdr->common.computerNameUnicode = (unicodeFlag & 1); + rdpdr->common.computerNameUnicode = ((unicodeFlag & 1) != 0) ? TRUE : FALSE; Stream_Read_UINT32(s, codePage); WINPR_UNUSED(codePage); /* Field is ignored */ From 3a2a65bbed48b6744604e0c39a09aa6248e46244 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:26:48 +0200 Subject: [PATCH 16/28] [server,shadow] fix integer narrow --- server/shadow/shadow_audin.c | 5 ++++- server/shadow/shadow_server.c | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/server/shadow/shadow_audin.c b/server/shadow/shadow_audin.c index 4f1a30e61..a95fb03d0 100644 --- a/server/shadow/shadow_audin.c +++ b/server/shadow/shadow_audin.c @@ -73,7 +73,10 @@ BOOL shadow_client_audin_init(rdpShadowClient* client) if (client->subsystem->audinFormats) { - if (!audin_server_set_formats(client->audin, client->subsystem->nAudinFormats, + if (client->subsystem->nAudinFormats > SSIZE_MAX) + goto fail; + + if (!audin_server_set_formats(client->audin, (SSIZE_T)client->subsystem->nAudinFormats, client->subsystem->audinFormats)) goto fail; } diff --git a/server/shadow/shadow_server.c b/server/shadow/shadow_server.c index bc72637fd..5ec0e0d90 100644 --- a/server/shadow/shadow_server.c +++ b/server/shadow/shadow_server.c @@ -801,7 +801,8 @@ static BOOL shadow_server_create_certificate(rdpShadowServer* server, const char if (!makecert) goto out_fail; - if (makecert_context_process(makecert, makecert_argc, makecert_argv) < 0) + WINPR_ASSERT(makecert_argc <= INT_MAX); + if (makecert_context_process(makecert, (int)makecert_argc, makecert_argv) < 0) goto out_fail; if (makecert_context_set_output_file_name(makecert, "shadow") != 1) From 6c25607c76eb49d5b69e095bd5028d1aa17ad535 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:30:40 +0200 Subject: [PATCH 17/28] [client,common] fix integer narrow --- client/common/test/TestClientCmdLine.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/common/test/TestClientCmdLine.c b/client/common/test/TestClientCmdLine.c index d7ed102c4..8bc7d7862 100644 --- a/client/common/test/TestClientCmdLine.c +++ b/client/common/test/TestClientCmdLine.c @@ -46,7 +46,10 @@ static INLINE BOOL testcase(const char* name, char** argv, size_t argc, int expe int status = 0; BOOL valid_settings = TRUE; rdpSettings* settings = freerdp_settings_new(0); - print_test_title(argc, argv); + + WINPR_ASSERT(argc <= INT_MAX); + + print_test_title((int)argc, argv); if (!settings) { @@ -54,7 +57,7 @@ static INLINE BOOL testcase(const char* name, char** argv, size_t argc, int expe return FALSE; } - status = freerdp_client_settings_parse_command_line(settings, argc, argv, FALSE); + status = freerdp_client_settings_parse_command_line(settings, (int)argc, argv, FALSE); if (validate_settings) { From 7037a8a84670161e5ad866cd76a2983fe42b3e57 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:47:12 +0200 Subject: [PATCH 18/28] [client,sample] fix integer narrow --- client/Sample/tf_freerdp.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/client/Sample/tf_freerdp.c b/client/Sample/tf_freerdp.c index 8050e5cb9..efffdc36d 100644 --- a/client/Sample/tf_freerdp.c +++ b/client/Sample/tf_freerdp.c @@ -382,16 +382,16 @@ static int RdpClientEntry(RDP_CLIENT_ENTRY_POINTS* pEntryPoints) int main(int argc, char* argv[]) { int rc = -1; - DWORD status = 0; - RDP_CLIENT_ENTRY_POINTS clientEntryPoints; - rdpContext* context = NULL; + RDP_CLIENT_ENTRY_POINTS clientEntryPoints = { 0 }; + RdpClientEntry(&clientEntryPoints); - context = freerdp_client_context_new(&clientEntryPoints); + rdpContext* context = freerdp_client_context_new(&clientEntryPoints); if (!context) goto fail; - status = freerdp_client_settings_parse_command_line(context->settings, argc, argv, FALSE); + const int status = + freerdp_client_settings_parse_command_line(context->settings, argc, argv, FALSE); if (status) { rc = freerdp_client_settings_command_line_status_print(context->settings, status, argc, @@ -405,7 +405,8 @@ int main(int argc, char* argv[]) if (freerdp_client_start(context) != 0) goto fail; - rc = tf_client_thread_proc(context->instance); + const DWORD res = tf_client_thread_proc(context->instance); + rc = (int)res; if (freerdp_client_stop(context) != 0) rc = -1; From 15141385f6448fb730527baaba509d14557ac958 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:21:23 +0200 Subject: [PATCH 19/28] [client,x11] fix integer narrow --- client/X11/xf_client.c | 3 ++- client/X11/xfreerdp.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 498a84950..70ba5b0cc 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -101,6 +101,7 @@ #include "xf_graphics.h" #include "xf_keyboard.h" #include "xf_channels.h" +#include "xf_client.h" #include "xfreerdp.h" #include "xf_utils.h" @@ -1689,7 +1690,7 @@ end: return exit_code; } -DWORD xf_exit_code_from_disconnect_reason(DWORD reason) +int xf_exit_code_from_disconnect_reason(DWORD reason) { if (reason == 0 || (reason >= XF_EXIT_PARSE_ARGUMENTS && reason <= XF_EXIT_CONNECT_NO_OR_MISSING_CREDENTIALS)) diff --git a/client/X11/xfreerdp.h b/client/X11/xfreerdp.h index 07a813154..e25c0199d 100644 --- a/client/X11/xfreerdp.h +++ b/client/X11/xfreerdp.h @@ -406,6 +406,6 @@ void xf_draw_screen_(xfContext* xfc, int x, int y, int w, int h, const char* fkt BOOL xf_keyboard_update_modifier_map(xfContext* xfc); -DWORD xf_exit_code_from_disconnect_reason(DWORD reason); +int xf_exit_code_from_disconnect_reason(DWORD reason); #endif /* FREERDP_CLIENT_X11_FREERDP_H */ From 911ed13efce61806301fff21c9c6d48c2e07a702 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:42:15 +0200 Subject: [PATCH 20/28] [channels,rdpei] fix integer narrow --- channels/rdpei/rdpei_common.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/channels/rdpei/rdpei_common.c b/channels/rdpei/rdpei_common.c index 4fc826543..a253c462b 100644 --- a/channels/rdpei/rdpei_common.c +++ b/channels/rdpei/rdpei_common.c @@ -93,7 +93,7 @@ BOOL rdpei_read_2byte_signed(wStream* s, INT16* value) negative = (byte & 0x40) ? TRUE : FALSE; - *value = (byte & 0x3F); + const BYTE val = (byte & 0x3F); if (byte & 0x80) { @@ -101,8 +101,10 @@ BOOL rdpei_read_2byte_signed(wStream* s, INT16* value) return FALSE; Stream_Read_UINT8(s, byte); - *value = ((*value & 0xFF) << 8) | byte; + *value = (INT16)((val << 8) | byte); } + else + *value = val; if (negative) *value *= -1; From 2c7ca7f4b2d7b7109b97a55e356fbaaffe316fc3 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 03:53:56 +0200 Subject: [PATCH 21/28] [rdtk] fix integer narrow --- rdtk/librdtk/rdtk_nine_patch.c | 35 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/rdtk/librdtk/rdtk_nine_patch.c b/rdtk/librdtk/rdtk_nine_patch.c index 7ccbf8fff..d070b2533 100644 --- a/rdtk/librdtk/rdtk_nine_patch.c +++ b/rdtk/librdtk/rdtk_nine_patch.c @@ -78,18 +78,6 @@ static int rdtk_image_copy_alpha_blend(uint8_t* pDstData, int nDstStep, int nXDs int rdtk_nine_patch_draw(rdtkSurface* surface, int nXDst, int nYDst, int nWidth, int nHeight, rdtkNinePatch* ninePatch) { - int x = 0; - int y = 0; - int width = 0; - int height = 0; - int nXSrc = 0; - int nYSrc = 0; - int nSrcStep = 0; - int nDstStep = 0; - uint8_t* pSrcData = NULL; - uint8_t* pDstData = NULL; - int scaleWidth = 0; - WINPR_ASSERT(surface); WINPR_ASSERT(ninePatch); @@ -101,19 +89,20 @@ int rdtk_nine_patch_draw(rdtkSurface* surface, int nXDst, int nYDst, int nWidth, WINPR_UNUSED(nHeight); - scaleWidth = nWidth - (ninePatch->width - ninePatch->scaleWidth); - nSrcStep = ninePatch->scanline; - pSrcData = ninePatch->data; - pDstData = surface->data; - nDstStep = surface->scanline; + int scaleWidth = nWidth - (ninePatch->width - ninePatch->scaleWidth); + int nSrcStep = ninePatch->scanline; + const uint8_t* pSrcData = ninePatch->data; + uint8_t* pDstData = surface->data; + WINPR_ASSERT(surface->scanline <= INT_MAX); + int nDstStep = (int)surface->scanline; /* top */ - x = 0; - y = 0; + int x = 0; + int y = 0; /* top left */ - nXSrc = 0; - nYSrc = 0; - width = ninePatch->scaleLeft; - height = ninePatch->scaleTop; + int nXSrc = 0; + int nYSrc = 0; + int width = ninePatch->scaleLeft; + int height = ninePatch->scaleTop; rdtk_image_copy_alpha_blend(pDstData, nDstStep, nXDst + x, nYDst + y, width, height, pSrcData, nSrcStep, nXSrc, nYSrc); x += width; From 315f79307899514aa84fcf99d76eb50a96211489 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 05:17:32 +0200 Subject: [PATCH 22/28] [common,settings] fix integer narrowing --- libfreerdp/common/settings.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 6e483942e..8cc575d4c 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -1212,26 +1212,30 @@ BOOL freerdp_settings_set_value_for_name(rdpSettings* settings, const char* name case RDP_SETTINGS_TYPE_UINT16: if (!value_to_uint(value, &uval, 0, UINT16_MAX)) return parsing_fail(name, "UINT16", value); - if (!freerdp_settings_set_uint16(settings, (FreeRDP_Settings_Keys_UInt16)index, uval)) + if (!freerdp_settings_set_uint16(settings, (FreeRDP_Settings_Keys_UInt16)index, + (UINT16)uval)) return parsing_fail(name, "UINT16", value); return TRUE; case RDP_SETTINGS_TYPE_INT16: if (!value_to_int(value, &ival, INT16_MIN, INT16_MAX)) return parsing_fail(name, "INT16", value); - if (!freerdp_settings_set_int16(settings, (FreeRDP_Settings_Keys_Int16)index, ival)) + if (!freerdp_settings_set_int16(settings, (FreeRDP_Settings_Keys_Int16)index, + (INT16)ival)) return parsing_fail(name, "INT16", value); return TRUE; case RDP_SETTINGS_TYPE_UINT32: if (!value_to_uint(value, &uval, 0, UINT32_MAX)) return parsing_fail(name, "UINT32", value); - if (!freerdp_settings_set_uint32(settings, (FreeRDP_Settings_Keys_UInt32)index, uval)) + if (!freerdp_settings_set_uint32(settings, (FreeRDP_Settings_Keys_UInt32)index, + (UINT32)uval)) return parsing_fail(name, "UINT32", value); return TRUE; case RDP_SETTINGS_TYPE_INT32: if (!value_to_int(value, &ival, INT32_MIN, INT32_MAX)) return parsing_fail(name, "INT32", value); - if (!freerdp_settings_set_int32(settings, (FreeRDP_Settings_Keys_Int32)index, ival)) + if (!freerdp_settings_set_int32(settings, (FreeRDP_Settings_Keys_Int32)index, + (INT32)ival)) return parsing_fail(name, "INT32", value); return TRUE; case RDP_SETTINGS_TYPE_UINT64: From a54a602dcc92f7c1613b052e04e8efd99e322dd0 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 05:11:58 +0200 Subject: [PATCH 23/28] [crypto,certificate] fix stackof handling --- libfreerdp/crypto/certificate.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index d15bf4420..f55eafd95 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -1172,7 +1172,15 @@ void certificate_free_int(rdpCertificate* cert) if (cert->x509) X509_free(cert->x509); if (cert->chain) + { + const int count = sk_X509_num(cert->chain); + for (int x = 0; x < count; x++) + { + X509* cur = sk_X509_value(cert->chain, x); + X509_free(cur); + } sk_X509_free(cert->chain); + } certificate_free_x509_certificate_chain(&cert->x509_cert_chain); cert_info_free(&cert->cert_info); @@ -1281,7 +1289,7 @@ rdpCertificate* freerdp_certificate_new_from_x509(const X509* xcert, const STACK if (chain) { - cert->chain = sk_X509_dup(chain); + cert->chain = sk_X509_deep_copy(chain, X509_dup, X509_free); } return cert; fail: From 46c1ae145c7e63caf196f6e3a3a9c93005bbe47f Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 05:23:14 +0200 Subject: [PATCH 24/28] [crypto,certificate] fix integer narrowing --- libfreerdp/crypto/certificate.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index f55eafd95..e27e6542f 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -525,9 +525,12 @@ static BOOL update_x509_from_info(rdpCertificate* cert) if (!mod || !e) goto fail; - if (!BN_bin2bn(info->Modulus, info->ModulusLength, mod)) + + WINPR_ASSERT(info->ModulusLength <= INT_MAX); + if (!BN_bin2bn(info->Modulus, (int)info->ModulusLength, mod)) goto fail; - if (!BN_bin2bn(info->exponent, sizeof(info->exponent), e)) + + if (!BN_bin2bn(info->exponent, (int)sizeof(info->exponent), e)) goto fail; #if !defined(OPENSSL_VERSION_MAJOR) || (OPENSSL_VERSION_MAJOR < 3) @@ -936,7 +939,12 @@ SSIZE_T freerdp_certificate_write_server_cert(const rdpCertificate* certificate, } const size_t end = Stream_GetPosition(s); - return end - start; + if (start > end) + return -1; + + const size_t diff = end - start; + WINPR_ASSERT(diff <= SSIZE_MAX); + return (SSIZE_T)diff; } /** @@ -1258,10 +1266,10 @@ rdpCertificate* freerdp_certificate_new_from_der(const BYTE* data, size_t length { rdpCertificate* cert = freerdp_certificate_new(); - if (!cert || !data || (length == 0)) + if (!cert || !data || (length == 0) || (length > INT_MAX)) goto fail; const BYTE* ptr = data; - cert->x509 = d2i_X509(NULL, &ptr, length); + cert->x509 = d2i_X509(NULL, &ptr, (int)length); if (!cert->x509) goto fail; if (!freerdp_rsa_from_x509(cert)) @@ -1399,8 +1407,9 @@ static BOOL bio_read_pem(BIO* bio, char** ppem, size_t* plength) WINPR_ASSERT(bio); WINPR_ASSERT(ppem); + const size_t blocksize = 2048; size_t offset = 0; - size_t length = 2048; + size_t length = blocksize; char* pem = NULL; while (offset < length) { @@ -1411,7 +1420,7 @@ static BOOL bio_read_pem(BIO* bio, char** ppem, size_t* plength) ERR_clear_error(); - const int status = BIO_read(bio, &pem[offset], length - offset); + const int status = BIO_read(bio, &pem[offset], (int)(length - offset)); if (status < 0) { WLog_ERR(TAG, "failed to read certificate"); @@ -1424,7 +1433,7 @@ static BOOL bio_read_pem(BIO* bio, char** ppem, size_t* plength) offset += (size_t)status; if (length - offset > 0) break; - length *= 2; + length += blocksize; } pem[offset] = '\0'; *ppem = pem; From 31570e31e1859c3d7b32fde8659dfbe49c4b283e Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 05:35:51 +0200 Subject: [PATCH 25/28] [crypto] fix integer narrowing --- libfreerdp/crypto/crypto.c | 6 ++-- libfreerdp/crypto/privatekey.c | 11 +++++-- libfreerdp/crypto/tls.c | 35 +++++++++++----------- libfreerdp/crypto/x509_utils.c | 53 ++++++++++++++++------------------ 4 files changed, 54 insertions(+), 51 deletions(-) diff --git a/libfreerdp/crypto/crypto.c b/libfreerdp/crypto/crypto.c index adbbd6cd2..806f9b986 100644 --- a/libfreerdp/crypto/crypto.c +++ b/libfreerdp/crypto/crypto.c @@ -94,12 +94,12 @@ static SSIZE_T crypto_rsa_common(const BYTE* input, size_t length, UINT32 key_le if (!(y = BN_new())) goto fail; - if (!BN_bin2bn(modulus_reverse, key_length, mod)) + if (!BN_bin2bn(modulus_reverse, (int)key_length, mod)) goto fail; - if (!BN_bin2bn(exponent_reverse, exponent_size, exp)) + if (!BN_bin2bn(exponent_reverse, (int)exponent_size, exp)) goto fail; - if (!BN_bin2bn(input_reverse, length, x)) + if (!BN_bin2bn(input_reverse, (int)length, x)) goto fail; if (BN_mod_exp(y, x, exp, mod, ctx) != 1) goto fail; diff --git a/libfreerdp/crypto/privatekey.c b/libfreerdp/crypto/privatekey.c index 9a6fb25fd..09d1593ac 100644 --- a/libfreerdp/crypto/privatekey.c +++ b/libfreerdp/crypto/privatekey.c @@ -125,7 +125,11 @@ static EVP_PKEY* evp_pkey_utils_from_pem(const char* data, size_t len, BOOL from if (fromFile) bio = BIO_new_file(data, "rb"); else - bio = BIO_new_mem_buf(data, len); + { + if (len > INT_MAX) + return NULL; + bio = BIO_new_mem_buf(data, (int)len); + } if (!bio) { @@ -426,7 +430,10 @@ BOOL freerdp_key_generate(rdpPrivateKey* key, size_t key_length) if (EVP_PKEY_keygen_init(pctx) != 1) goto fail; - if (EVP_PKEY_CTX_set_rsa_keygen_bits(pctx, key_length) != 1) + if (key_length > INT_MAX) + goto fail; + + if (EVP_PKEY_CTX_set_rsa_keygen_bits(pctx, (int)key_length) != 1) goto fail; EVP_PKEY_free(key->evp); diff --git a/libfreerdp/crypto/tls.c b/libfreerdp/crypto/tls.c index ef3182882..4047cc525 100644 --- a/libfreerdp/crypto/tls.c +++ b/libfreerdp/crypto/tls.c @@ -240,16 +240,14 @@ static int bio_rdp_tls_read(BIO* bio, char* buf, int size) static int bio_rdp_tls_puts(BIO* bio, const char* str) { - size_t size = 0; - int status = 0; - if (!str) return 0; - size = strlen(str); + const size_t size = strnlen(str, INT_MAX + 1UL); + if (size > INT_MAX) + return -1; ERR_clear_error(); - status = BIO_write(bio, str, size); - return status; + return BIO_write(bio, str, (int)size); } static int bio_rdp_tls_gets(BIO* bio, char* str, int size) @@ -262,7 +260,7 @@ static long bio_rdp_tls_ctrl(BIO* bio, int cmd, long num, void* ptr) BIO* ssl_rbio = NULL; BIO* ssl_wbio = NULL; BIO* next_bio = NULL; - int status = -1; + long status = -1; BIO_RDP_TLS* tls = (BIO_RDP_TLS*)BIO_get_data(bio); if (!tls) @@ -1087,7 +1085,7 @@ TlsHandshakeResult freerdp_tls_accept_ex(rdpTls* tls, BIO* underlying, rdpSettin { WINPR_ASSERT(tls); - long options = 0; + int options = 0; int status = 0; /** @@ -1239,33 +1237,34 @@ BOOL freerdp_tls_send_alert(rdpTls* tls) int freerdp_tls_write_all(rdpTls* tls, const BYTE* data, int length) { WINPR_ASSERT(tls); - int status = 0; int offset = 0; BIO* bio = tls->bio; while (offset < length) { ERR_clear_error(); - status = BIO_write(bio, &data[offset], length - offset); + const int rc = BIO_write(bio, &data[offset], length - offset); - if (status > 0) - { - offset += status; - } + if (rc < 0) + return rc; + + if (rc > 0) + offset += rc; else { if (!BIO_should_retry(bio)) return -1; if (BIO_write_blocked(bio)) - status = BIO_wait_write(bio, 100); + { + const long status = BIO_wait_write(bio, 100); + if (status < 0) + return -1; + } else if (BIO_read_blocked(bio)) return -2; /* Abort write, there is data that must be read */ else USleep(100); - - if (status < 0) - return -1; } } diff --git a/libfreerdp/crypto/x509_utils.c b/libfreerdp/crypto/x509_utils.c index f362e06bb..d4226b72e 100644 --- a/libfreerdp/crypto/x509_utils.c +++ b/libfreerdp/crypto/x509_utils.c @@ -616,11 +616,7 @@ out_free_issuer: static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, size_t* plength) { - BIO* bio = NULL; - int status = 0; int count = 0; - size_t offset = 0; - size_t length = 0; BOOL rc = FALSE; BYTE* pemCert = NULL; @@ -631,7 +627,7 @@ static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, * Don't manage certificates internally, leave it up entirely to the external client * implementation */ - bio = BIO_new(BIO_s_mem()); + BIO* bio = BIO_new(BIO_s_mem()); if (!bio) { @@ -640,7 +636,7 @@ static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, } X509* wcert = WINPR_CAST_CONST_PTR_AWAY(xcert, X509*); - status = PEM_write_bio_X509(bio, wcert); + int status = PEM_write_bio_X509(bio, wcert); if (status < 0) { @@ -663,46 +659,42 @@ static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, } } - offset = 0; - length = 2048; + const size_t blocksize = 2048; + size_t offset = 0; + size_t length = blocksize; pemCert = (BYTE*)malloc(length + 1); - if (!pemCert) + if (!pemCert || (length > INT_MAX)) { WLog_ERR(TAG, "error allocating pemCert"); goto fail; } - ERR_clear_error(); - status = BIO_read(bio, pemCert, length); - - if (status < 0) + while (offset < length) { - WLog_ERR(TAG, "failed to read certificate"); - goto fail; - } - - offset += (size_t)status; - - while (offset >= length) - { - int new_len = 0; - BYTE* new_cert = NULL; - new_len = length * 2; - new_cert = (BYTE*)realloc(pemCert, new_len + 1); + size_t new_len = length + blocksize; + BYTE* new_cert = (BYTE*)realloc(pemCert, new_len + 1); + size_t diff = length - offset; if (!new_cert) goto fail; length = new_len; pemCert = new_cert; + ERR_clear_error(); - status = BIO_read(bio, &pemCert[offset], length - offset); + + if (diff > INT_MAX) + goto fail; + + status = BIO_read(bio, &pemCert[offset], (int)diff); if (status < 0) break; - offset += status; + offset += (size_t)status; + if ((size_t)status < diff) + break; } if (status < 0) @@ -735,7 +727,12 @@ X509* x509_utils_from_pem(const char* data, size_t len, BOOL fromFile) if (fromFile) bio = BIO_new_file(data, "rb"); else - bio = BIO_new_mem_buf(data, len); + { + if (len > INT_MAX) + return NULL; + + bio = BIO_new_mem_buf(data, (int)len); + } if (!bio) { From ac1a92277492e5d02eb6e087060bb7b8921936e8 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 1 Oct 2024 10:00:25 +0200 Subject: [PATCH 26/28] [common,settings] annotate freerdp_settings_set_value_for_name we cast a SSIZE_T to the required enum value. Since we do not know the enum contains any value at all it might lead to compiler/analyzer warning us about that. Silence the warning as we can not avoid it. --- libfreerdp/common/settings.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 8cc575d4c..7bace87d8 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -1207,11 +1207,15 @@ BOOL freerdp_settings_set_value_for_name(rdpSettings* settings, const char* name (_strnicmp(value, "FALSE", 6) == 0) || (_strnicmp(value, "OFF", 6) == 0); if (!val && !nval) return parsing_fail(name, "BOOL", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) return freerdp_settings_set_bool(settings, (FreeRDP_Settings_Keys_Bool)index, val); } case RDP_SETTINGS_TYPE_UINT16: if (!value_to_uint(value, &uval, 0, UINT16_MAX)) return parsing_fail(name, "UINT16", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) if (!freerdp_settings_set_uint16(settings, (FreeRDP_Settings_Keys_UInt16)index, (UINT16)uval)) return parsing_fail(name, "UINT16", value); @@ -1220,6 +1224,8 @@ BOOL freerdp_settings_set_value_for_name(rdpSettings* settings, const char* name case RDP_SETTINGS_TYPE_INT16: if (!value_to_int(value, &ival, INT16_MIN, INT16_MAX)) return parsing_fail(name, "INT16", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) if (!freerdp_settings_set_int16(settings, (FreeRDP_Settings_Keys_Int16)index, (INT16)ival)) return parsing_fail(name, "INT16", value); @@ -1227,6 +1233,8 @@ BOOL freerdp_settings_set_value_for_name(rdpSettings* settings, const char* name case RDP_SETTINGS_TYPE_UINT32: if (!value_to_uint(value, &uval, 0, UINT32_MAX)) return parsing_fail(name, "UINT32", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) if (!freerdp_settings_set_uint32(settings, (FreeRDP_Settings_Keys_UInt32)index, (UINT32)uval)) return parsing_fail(name, "UINT32", value); @@ -1234,6 +1242,8 @@ BOOL freerdp_settings_set_value_for_name(rdpSettings* settings, const char* name case RDP_SETTINGS_TYPE_INT32: if (!value_to_int(value, &ival, INT32_MIN, INT32_MAX)) return parsing_fail(name, "INT32", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) if (!freerdp_settings_set_int32(settings, (FreeRDP_Settings_Keys_Int32)index, (INT32)ival)) return parsing_fail(name, "INT32", value); @@ -1241,17 +1251,22 @@ BOOL freerdp_settings_set_value_for_name(rdpSettings* settings, const char* name case RDP_SETTINGS_TYPE_UINT64: if (!value_to_uint(value, &uval, 0, UINT64_MAX)) return parsing_fail(name, "UINT64", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) if (!freerdp_settings_set_uint64(settings, (FreeRDP_Settings_Keys_UInt64)index, uval)) return parsing_fail(name, "UINT64", value); return TRUE; case RDP_SETTINGS_TYPE_INT64: if (!value_to_int(value, &ival, INT64_MIN, INT64_MAX)) return parsing_fail(name, "INT64", value); + + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) if (!freerdp_settings_set_int64(settings, (FreeRDP_Settings_Keys_Int64)index, ival)) return parsing_fail(name, "INT64", value); return TRUE; case RDP_SETTINGS_TYPE_STRING: + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) return freerdp_settings_set_string(settings, (FreeRDP_Settings_Keys_String)index, value); case RDP_SETTINGS_TYPE_POINTER: From 830f208ec4cce52c5df13f9057983298c4bc857f Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 3 Oct 2024 18:13:52 +0200 Subject: [PATCH 27/28] [cyrpto,x509] delete unused function --- libfreerdp/crypto/x509_utils.c | 106 --------------------------------- 1 file changed, 106 deletions(-) diff --git a/libfreerdp/crypto/x509_utils.c b/libfreerdp/crypto/x509_utils.c index d4226b72e..60a3c07c3 100644 --- a/libfreerdp/crypto/x509_utils.c +++ b/libfreerdp/crypto/x509_utils.c @@ -614,112 +614,6 @@ out_free_issuer: free(subject); } -static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, size_t* plength) -{ - int count = 0; - BOOL rc = FALSE; - BYTE* pemCert = NULL; - - if (!xcert || !plength) - return NULL; - - /** - * Don't manage certificates internally, leave it up entirely to the external client - * implementation - */ - BIO* bio = BIO_new(BIO_s_mem()); - - if (!bio) - { - WLog_ERR(TAG, "BIO_new() failure"); - return NULL; - } - - X509* wcert = WINPR_CAST_CONST_PTR_AWAY(xcert, X509*); - int status = PEM_write_bio_X509(bio, wcert); - - if (status < 0) - { - WLog_ERR(TAG, "PEM_write_bio_X509 failure: %d", status); - goto fail; - } - - if (chain) - { - count = sk_X509_num(chain); - for (int x = 0; x < count; x++) - { - X509* c = sk_X509_value(chain, x); - status = PEM_write_bio_X509(bio, c); - if (status < 0) - { - WLog_ERR(TAG, "PEM_write_bio_X509 failure: %d", status); - goto fail; - } - } - } - - const size_t blocksize = 2048; - size_t offset = 0; - size_t length = blocksize; - pemCert = (BYTE*)malloc(length + 1); - - if (!pemCert || (length > INT_MAX)) - { - WLog_ERR(TAG, "error allocating pemCert"); - goto fail; - } - - while (offset < length) - { - size_t new_len = length + blocksize; - BYTE* new_cert = (BYTE*)realloc(pemCert, new_len + 1); - - size_t diff = length - offset; - if (!new_cert) - goto fail; - - length = new_len; - pemCert = new_cert; - - ERR_clear_error(); - - if (diff > INT_MAX) - goto fail; - - status = BIO_read(bio, &pemCert[offset], (int)diff); - - if (status < 0) - break; - - offset += (size_t)status; - if ((size_t)status < diff) - break; - } - - if (status < 0) - { - WLog_ERR(TAG, "failed to read certificate"); - goto fail; - } - - length = offset; - pemCert[length] = '\0'; - *plength = length; - rc = TRUE; -fail: - - if (!rc) - { - WLog_ERR(TAG, "Failed to extract PEM from certificate %p", xcert); - free(pemCert); - pemCert = NULL; - } - - BIO_free_all(bio); - return pemCert; -} - X509* x509_utils_from_pem(const char* data, size_t len, BOOL fromFile) { X509* x509 = NULL; From e14b7a1f29ed25e6ccb961e9e4e4fdcfefe721a3 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 3 Oct 2024 18:09:01 +0200 Subject: [PATCH 28/28] [crypto,cert] cleanup cert chain duplication/cleanup --- libfreerdp/crypto/certificate.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index e27e6542f..faab86d8e 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -46,6 +46,7 @@ #include #include #include +#include #endif #include "certificate.h" @@ -1180,15 +1181,7 @@ void certificate_free_int(rdpCertificate* cert) if (cert->x509) X509_free(cert->x509); if (cert->chain) - { - const int count = sk_X509_num(cert->chain); - for (int x = 0; x < count; x++) - { - X509* cur = sk_X509_value(cert->chain, x); - X509_free(cur); - } - sk_X509_free(cert->chain); - } + sk_X509_pop_free(cert->chain, X509_free); certificate_free_x509_certificate_chain(&cert->x509_cert_chain); cert_info_free(&cert->cert_info); @@ -1296,9 +1289,8 @@ rdpCertificate* freerdp_certificate_new_from_x509(const X509* xcert, const STACK goto fail; if (chain) - { - cert->chain = sk_X509_deep_copy(chain, X509_dup, X509_free); - } + cert->chain = X509_chain_up_ref(chain); + return cert; fail: freerdp_certificate_free(cert);