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; 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; 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 */ diff --git a/client/common/test/TestClientCmdLine.c b/client/common/test/TestClientCmdLine.c index 36de79823..79ee06c5c 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) { diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 6e483942e..7bace87d8 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -1207,47 +1207,66 @@ 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); - if (!freerdp_settings_set_uint16(settings, (FreeRDP_Settings_Keys_UInt16)index, uval)) + + // 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); 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)) + + // 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); 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)) + + // 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); 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)) + + // 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); return TRUE; 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: diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index d15bf4420..faab86d8e 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -46,6 +46,7 @@ #include #include #include +#include #endif #include "certificate.h" @@ -525,9 +526,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 +940,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; } /** @@ -1172,7 +1181,7 @@ void certificate_free_int(rdpCertificate* cert) if (cert->x509) X509_free(cert->x509); if (cert->chain) - 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); @@ -1250,10 +1259,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)) @@ -1280,9 +1289,8 @@ rdpCertificate* freerdp_certificate_new_from_x509(const X509* xcert, const STACK goto fail; if (chain) - { - cert->chain = sk_X509_dup(chain); - } + cert->chain = X509_chain_up_ref(chain); + return cert; fail: freerdp_certificate_free(cert); @@ -1391,8 +1399,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) { @@ -1403,7 +1412,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"); @@ -1416,7 +1425,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; 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 1c7a60c6b..7693aa34f 100644 --- a/libfreerdp/crypto/tls.c +++ b/libfreerdp/crypto/tls.c @@ -242,16 +242,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) @@ -264,7 +262,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) @@ -1089,7 +1087,7 @@ TlsHandshakeResult freerdp_tls_accept_ex(rdpTls* tls, BIO* underlying, rdpSettin { WINPR_ASSERT(tls); - long options = 0; + int options = 0; int status = 0; /** @@ -1241,33 +1239,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..60a3c07c3 100644 --- a/libfreerdp/crypto/x509_utils.c +++ b/libfreerdp/crypto/x509_utils.c @@ -614,120 +614,6 @@ out_free_issuer: free(subject); } -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; - - if (!xcert || !plength) - return NULL; - - /** - * Don't manage certificates internally, leave it up entirely to the external client - * implementation - */ - 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*); - 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; - } - } - } - - offset = 0; - length = 2048; - pemCert = (BYTE*)malloc(length + 1); - - if (!pemCert) - { - WLog_ERR(TAG, "error allocating pemCert"); - goto fail; - } - - ERR_clear_error(); - status = BIO_read(bio, pemCert, length); - - if (status < 0) - { - 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); - - if (!new_cert) - goto fail; - - length = new_len; - pemCert = new_cert; - ERR_clear_error(); - status = BIO_read(bio, &pemCert[offset], length - offset); - - if (status < 0) - break; - - offset += status; - } - - 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; @@ -735,7 +621,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) { 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; 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); 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; 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 */ 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"); 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); 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 063c72f85..a47d4a568 100644 --- a/server/shadow/shadow_server.c +++ b/server/shadow/shadow_server.c @@ -804,7 +804,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) 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) 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]; 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) { 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; 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)) { 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)) { 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) diff --git a/winpr/libwinpr/timezone/timezone.c b/winpr/libwinpr/timezone/timezone.c index a7c582be2..2476f9479 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) 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; 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; } }