From 343900d227f693d40a98a12a117191f6b9b0f1fd Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 08:57:05 +0200 Subject: [PATCH 01/10] [crypto,cert] make a deep copy of the certificate chain The parameters of freerdp_certificate_new_from_x509 are const, so only work with a copy of the input. --- libfreerdp/crypto/certificate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index faab86d8e..419d97357 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -1289,7 +1289,7 @@ rdpCertificate* freerdp_certificate_new_from_x509(const X509* xcert, const STACK goto fail; if (chain) - cert->chain = X509_chain_up_ref(chain); + cert->chain = sk_X509_deep_copy(chain, X509_dup, X509_free); return cert; fail: From 1cdc864c7ddc1bb1db8e9dd86ea725d935acfeb8 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:22:54 +0200 Subject: [PATCH 02/10] [warnings] fix integer narrowing --- winpr/libwinpr/thread/process.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/winpr/libwinpr/thread/process.c b/winpr/libwinpr/thread/process.c index 62a937abe..53ba41767 100644 --- a/winpr/libwinpr/thread/process.c +++ b/winpr/libwinpr/thread/process.c @@ -263,7 +263,12 @@ static BOOL CreateProcessExA(HANDLE hToken, DWORD dwLogonFlags, LPCSTR lpApplica #ifdef F_MAXFD // on some BSD derivates maxfd = fcntl(0, F_MAXFD); #else - maxfd = sysconf(_SC_OPEN_MAX); + { + const long rc = sysconf(_SC_OPEN_MAX); + if ((rc < INT32_MIN) || (rc > INT32_MAX)) + goto finish; + maxfd = (int)rc; + } #endif for (int fd = 3; fd < maxfd; fd++) @@ -555,24 +560,26 @@ static int pidfd_open(pid_t pid) #define PIDFD_NONBLOCK O_NONBLOCK #endif /* PIDFD_NONBLOCK */ - int fd = syscall(__NR_pidfd_open, pid, PIDFD_NONBLOCK); + long fd = syscall(__NR_pidfd_open, pid, PIDFD_NONBLOCK); if (fd < 0 && errno == EINVAL) { /* possibly PIDFD_NONBLOCK is not supported, let's try to create a pidfd and set it * non blocking afterward */ int flags = 0; fd = syscall(__NR_pidfd_open, pid, 0); - if (fd < 0) + if ((fd < 0) || (fd > INT32_MAX)) return -1; - flags = fcntl(fd, F_GETFL); - if (flags < 0 || fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0) + flags = fcntl((int)fd, F_GETFL); + if ((flags < 0) || fcntl((int)fd, F_SETFL, flags | O_NONBLOCK) < 0) { - close(fd); + close((int)fd); fd = -1; } } - return fd; + if ((fd < 0) || (fd > INT32_MAX)) + return -1; + return (int)fd; #else return -1; #endif From bd28c2d4bfe61c62548ba418a896261fda22ceec Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:33:33 +0200 Subject: [PATCH 03/10] [warnings] fix integer narrowing --- winpr/libwinpr/crt/string.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/winpr/libwinpr/crt/string.c b/winpr/libwinpr/crt/string.c index d0bd38495..f5427a4ee 100644 --- a/winpr/libwinpr/crt/string.c +++ b/winpr/libwinpr/crt/string.c @@ -469,7 +469,7 @@ LPSTR CharUpperA(LPSTR lpsz) char c = *lpsz; if ((c >= 'a') && (c <= 'z')) - c = c - 'a' + 'A'; + c = (char)(c - 'a' + 'A'); *lpsz = c; return lpsz; @@ -478,7 +478,7 @@ LPSTR CharUpperA(LPSTR lpsz) for (size_t i = 0; i < length; i++) { if ((lpsz[i] >= 'a') && (lpsz[i] <= 'z')) - lpsz[i] = lpsz[i] - 'a' + 'A'; + lpsz[i] = (char)(lpsz[i] - 'a' + 'A'); } return lpsz; @@ -524,7 +524,7 @@ DWORD CharUpperBuffA(LPSTR lpsz, DWORD cchLength) for (DWORD i = 0; i < cchLength; i++) { if ((lpsz[i] >= 'a') && (lpsz[i] <= 'z')) - lpsz[i] = lpsz[i] - 'a' + 'A'; + lpsz[i] = (char)(lpsz[i] - 'a' + 'A'); } return cchLength; @@ -561,7 +561,7 @@ LPSTR CharLowerA(LPSTR lpsz) char c = *lpsz; if ((c >= 'A') && (c <= 'Z')) - c = c - 'A' + 'a'; + c = (char)(c - 'A' + 'a'); *lpsz = c; return lpsz; @@ -570,7 +570,7 @@ LPSTR CharLowerA(LPSTR lpsz) for (size_t i = 0; i < length; i++) { if ((lpsz[i] >= 'A') && (lpsz[i] <= 'Z')) - lpsz[i] = lpsz[i] - 'A' + 'a'; + lpsz[i] = (char)(lpsz[i] - 'A' + 'a'); } return lpsz; @@ -590,7 +590,7 @@ DWORD CharLowerBuffA(LPSTR lpsz, DWORD cchLength) for (DWORD i = 0; i < cchLength; i++) { if ((lpsz[i] >= 'A') && (lpsz[i] <= 'Z')) - lpsz[i] = lpsz[i] - 'A' + 'a'; + lpsz[i] = (char)(lpsz[i] - 'A' + 'a'); } return cchLength; From 62e556f4d04131a9580fcf3f6dbbcc37a6ff97db Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:34:25 +0200 Subject: [PATCH 04/10] [warnings] fix integer narrowing --- libfreerdp/primitives/prim_YCoCg.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libfreerdp/primitives/prim_YCoCg.c b/libfreerdp/primitives/prim_YCoCg.c index 089fb656a..a19075431 100644 --- a/libfreerdp/primitives/prim_YCoCg.c +++ b/libfreerdp/primitives/prim_YCoCg.c @@ -51,10 +51,10 @@ static pstatus_t general_YCoCgToRGB_8u_AC4R(const BYTE* pSrc, INT32 srcStep, BYT const INT16 Cg = convert(*sptr++, shift); const INT16 Co = convert(*sptr++, shift); const INT16 Y = *sptr++; /* UINT8->INT16 */ - const INT16 T = Y - Cg; - const INT16 B = T + Co; - const INT16 G = Y + Cg; - const INT16 R = T - Co; + const INT16 T = (INT16)(Y - Cg); + const INT16 B = (INT16)(T + Co); + const INT16 G = (INT16)(Y + Cg); + const INT16 R = (INT16)(T - Co); BYTE A = *sptr++; if (!withAlpha) From 15d408d6fca68954fced2903bc0caaa0453d7388 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:49:23 +0200 Subject: [PATCH 05/10] [warnings] fix integer narrowing --- server/proxy/pf_config.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/proxy/pf_config.c b/server/proxy/pf_config.c index 216d1e82f..42be9eb66 100644 --- a/server/proxy/pf_config.c +++ b/server/proxy/pf_config.c @@ -930,8 +930,10 @@ static BOOL pf_config_copy_string_list(char*** dst, size_t* size, char** src, si *dst = NULL; *size = 0; - if (srcSize == 0) - return TRUE; + if (srcSize > INT32_MAX) + return FALSE; + + if (srcSize != 0) { char* csv = CommandLineToCommaSeparatedValues(srcSize, src); *dst = CommandLineParseCommaSeparatedValues(csv, size); From e6cf35c518036cdd0c9ae28f3bad926156c2dd75 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:53:09 +0200 Subject: [PATCH 06/10] [warnings] fix integer narrowing --- server/shadow/X11/x11_shadow.c | 15 ++++++++++----- server/shadow/shadow_screen.c | 26 +++++++++++++------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/server/shadow/X11/x11_shadow.c b/server/shadow/X11/x11_shadow.c index 298c6db4e..c307569aa 100644 --- a/server/shadow/X11/x11_shadow.c +++ b/server/shadow/X11/x11_shadow.c @@ -722,14 +722,19 @@ static BOOL x11_shadow_check_resize(x11ShadowSubsystem* subsystem) /* Screen size changed. Refresh monitor definitions and trigger screen resize */ subsystem->common.numMonitors = x11_shadow_enum_monitors(subsystem->common.monitors, 16); - shadow_screen_resize(subsystem->common.server->screen); - subsystem->width = attr.width; - subsystem->height = attr.height; + if (!shadow_screen_resize(subsystem->common.server->screen)) + return FALSE; + + WINPR_ASSERT(attr.width > 0); + WINPR_ASSERT(attr.height > 0); + + subsystem->width = (UINT32)attr.width; + subsystem->height = (UINT32)attr.height; virtualScreen->left = 0; virtualScreen->top = 0; - virtualScreen->right = subsystem->width - 1; - virtualScreen->bottom = subsystem->height - 1; + virtualScreen->right = attr.width - 1; + virtualScreen->bottom = attr.height - 1; virtualScreen->flags = 1; return TRUE; } diff --git a/server/shadow/shadow_screen.c b/server/shadow/shadow_screen.c index 8a9e0b304..f5a44d0ba 100644 --- a/server/shadow/shadow_screen.c +++ b/server/shadow/shadow_screen.c @@ -119,23 +119,22 @@ void shadow_screen_free(rdpShadowScreen* screen) BOOL shadow_screen_resize(rdpShadowScreen* screen) { - int x = 0; - int y = 0; - int width = 0; - int height = 0; - MONITOR_DEF* primary = NULL; - rdpShadowSubsystem* subsystem = NULL; - if (!screen) return FALSE; - subsystem = screen->server->subsystem; - primary = &(subsystem->monitors[subsystem->selectedMonitor]); + WINPR_ASSERT(screen->server); - x = primary->left; - y = primary->top; - width = primary->right - primary->left + 1; - height = primary->bottom - primary->top + 1; + rdpShadowSubsystem* subsystem = screen->server->subsystem; + WINPR_ASSERT(subsystem); + WINPR_ASSERT(subsystem->monitors); + + MONITOR_DEF* primary = &(subsystem->monitors[subsystem->selectedMonitor]); + WINPR_ASSERT(primary); + + const INT32 x = primary->left; + const INT32 y = primary->top; + const INT32 width = primary->right - primary->left + 1; + const INT32 height = primary->bottom - primary->top + 1; WINPR_ASSERT(x >= 0); WINPR_ASSERT(x <= UINT16_MAX); @@ -145,6 +144,7 @@ BOOL shadow_screen_resize(rdpShadowScreen* screen) WINPR_ASSERT(width <= UINT16_MAX); WINPR_ASSERT(height >= 0); WINPR_ASSERT(height <= UINT16_MAX); + if (shadow_surface_resize(screen->primary, (UINT16)x, (UINT16)y, (UINT16)width, (UINT16)height) && shadow_surface_resize(screen->lobby, (UINT16)x, (UINT16)y, (UINT16)width, (UINT16)height)) From 8f069b2be447e6571530c2b98340f7cd34276688 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:57:40 +0200 Subject: [PATCH 07/10] [warnings] fix integer narrowing --- libfreerdp/cache/persistent.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libfreerdp/cache/persistent.c b/libfreerdp/cache/persistent.c index 01cfe6a87..063e8d0ee 100644 --- a/libfreerdp/cache/persistent.c +++ b/libfreerdp/cache/persistent.c @@ -32,7 +32,7 @@ struct rdp_persistent_cache { FILE* fp; BOOL write; - UINT32 version; + int version; int count; char* filename; BYTE* bmpData; @@ -81,7 +81,6 @@ static int persistent_cache_read_entry_v2(rdpPersistentCache* persistent, static int persistent_cache_write_entry_v2(rdpPersistentCache* persistent, const PERSISTENT_CACHE_ENTRY* entry) { - int padding = 0; PERSISTENT_CACHE_ENTRY_V2 entry2 = { 0 }; WINPR_ASSERT(persistent); @@ -95,17 +94,17 @@ static int persistent_cache_write_entry_v2(rdpPersistentCache* persistent, if (!entry2.flags) entry2.flags = 0x00000011; - if (fwrite((void*)&entry2, sizeof(entry2), 1, persistent->fp) != 1) + if (fwrite(&entry2, sizeof(entry2), 1, persistent->fp) != 1) return -1; - if (fwrite((void*)entry->data, entry->size, 1, persistent->fp) != 1) + if (fwrite(entry->data, entry->size, 1, persistent->fp) != 1) return -1; if (0x4000 > entry->size) { - padding = 0x4000 - entry->size; + const size_t padding = 0x4000 - entry->size; - if (fwrite((void*)persistent->bmpData, padding, 1, persistent->fp) != 1) + if (fwrite(persistent->bmpData, padding, 1, persistent->fp) != 1) return -1; } @@ -323,7 +322,8 @@ int persistent_cache_open(rdpPersistentCache* persistent, const char* filename, if (persistent->write) { - persistent->version = version; + WINPR_ASSERT(version <= INT32_MAX); + persistent->version = (int)version; return persistent_cache_open_write(persistent); } From dceb15d14ea73d05cd5fd1f362688160af95b0e5 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 10:58:47 +0200 Subject: [PATCH 08/10] [warnings] fix integer narrowing --- libfreerdp/common/settings.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 7bace87d8..264c64e0b 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -781,13 +781,16 @@ void freerdp_addin_argv_free(ADDIN_ARGV* args) ADDIN_ARGV* freerdp_addin_argv_new(size_t argc, const char* argv[]) { + if (argc > INT32_MAX) + return NULL; + ADDIN_ARGV* args = calloc(1, sizeof(ADDIN_ARGV)); if (!args) return NULL; if (argc == 0) return args; - args->argc = argc; + args->argc = (int)argc; args->argv = calloc(argc, sizeof(char*)); if (!args->argv) goto fail; From f0c1cbe20fae7d9ccd38fbeaa5e405c8e3af973d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 11:00:32 +0200 Subject: [PATCH 09/10] [warnings] fix integer narrowing --- libfreerdp/core/gcc.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libfreerdp/core/gcc.c b/libfreerdp/core/gcc.c index 8c088de06..0b9c835d6 100644 --- a/libfreerdp/core/gcc.c +++ b/libfreerdp/core/gcc.c @@ -2120,18 +2120,18 @@ BOOL gcc_read_client_monitor_data(wStream* s, rdpMcs* mcs) for (UINT32 index = 0; index < monitorCount; index++) { - UINT32 left = 0; - UINT32 top = 0; - UINT32 right = 0; - UINT32 bottom = 0; - UINT32 flags = 0; + INT32 left = 0; + INT32 top = 0; + INT32 right = 0; + INT32 bottom = 0; + INT32 flags = 0; rdpMonitor* current = &settings->MonitorDefArray[index]; - Stream_Read_UINT32(s, left); /* left */ - Stream_Read_UINT32(s, top); /* top */ - Stream_Read_UINT32(s, right); /* right */ - Stream_Read_UINT32(s, bottom); /* bottom */ - Stream_Read_UINT32(s, flags); /* flags */ + Stream_Read_INT32(s, left); /* left */ + Stream_Read_INT32(s, top); /* top */ + Stream_Read_INT32(s, right); /* right */ + Stream_Read_INT32(s, bottom); /* bottom */ + Stream_Read_INT32(s, flags); /* flags */ current->x = left; current->y = top; current->width = right - left + 1; From 026b5218ff9e0f894342dc91461c7fccd5715718 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 4 Oct 2024 11:11:09 +0200 Subject: [PATCH 10/10] [warnings] fix integer narrowing --- libfreerdp/core/transport.c | 54 ++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index 92611d285..a2cea0986 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -1038,7 +1038,10 @@ SSIZE_T transport_parse_pdu(rdpTransport* transport, wStream* s, BOOL* incomplet pduLength = parse_default_mode_pdu(transport, s); if (pduLength == 0) - return pduLength; + return 0; + + if (pduLength > SSIZE_MAX) + return -1; const size_t len = Stream_Length(s); if (len > pduLength) @@ -1047,7 +1050,7 @@ SSIZE_T transport_parse_pdu(rdpTransport* transport, wStream* s, BOOL* incomplet if (incomplete) *incomplete = len < pduLength; - return pduLength; + return (int)pduLength; } static int transport_default_read_pdu(rdpTransport* transport, wStream* s) @@ -1067,9 +1070,9 @@ static int transport_default_read_pdu(rdpTransport* transport, wStream* s) BYTE c = '\0'; do { - const int rc = transport_read_layer(transport, &c, 1); + const SSIZE_T rc = transport_read_layer(transport, &c, 1); if (rc != 1) - return rc; + return (rc == 0) ? 0 : -1; if (!Stream_EnsureRemainingCapacity(s, 1)) return -1; Stream_Write_UINT8(s, c); @@ -1079,9 +1082,9 @@ static int transport_default_read_pdu(rdpTransport* transport, wStream* s) { if (!Stream_EnsureCapacity(s, 4)) return -1; - const int rc = transport_read_layer_bytes(transport, s, 4); + const SSIZE_T rc = transport_read_layer_bytes(transport, s, 4); if (rc != 1) - return rc; + return (rc == 0) ? 0 : -1; } else { @@ -1089,12 +1092,13 @@ static int transport_default_read_pdu(rdpTransport* transport, wStream* s) status = transport_parse_pdu(transport, s, &incomplete); while ((status == 0) && incomplete) { - int rc = 0; if (!Stream_EnsureRemainingCapacity(s, 1)) return -1; - rc = transport_read_layer_bytes(transport, s, 1); + SSIZE_T rc = transport_read_layer_bytes(transport, s, 1); + if (rc > INT32_MAX) + return INT32_MAX; if (rc != 1) - return rc; + return (int)rc; status = transport_parse_pdu(transport, s, &incomplete); } @@ -1114,7 +1118,11 @@ static int transport_default_read_pdu(rdpTransport* transport, wStream* s) status = transport_read_layer_bytes(transport, s, pduLength - Stream_GetPosition(s)); if (status != 1) - return status; + { + if ((status < INT32_MIN) || (status > INT32_MAX)) + return -1; + return (int)status; + } if (Stream_GetPosition(s) >= pduLength) WLog_Packet(transport->log, WLOG_TRACE, Stream_Buffer(s), pduLength, @@ -1123,7 +1131,10 @@ static int transport_default_read_pdu(rdpTransport* transport, wStream* s) Stream_SealLength(s); Stream_SetPosition(s, 0); - return Stream_Length(s); + const size_t len = Stream_Length(s); + if (len > INT32_MAX) + return -1; + return (int)len; } int transport_write(rdpTransport* transport, wStream* s) @@ -1136,10 +1147,7 @@ int transport_write(rdpTransport* transport, wStream* s) static int transport_default_write(rdpTransport* transport, wStream* s) { - size_t length = 0; int status = -1; - int writtenlength = 0; - rdpRdp* rdp = NULL; rdpContext* context = transport_get_context(transport); WINPR_ASSERT(transport); @@ -1150,7 +1158,7 @@ static int transport_default_write(rdpTransport* transport, wStream* s) Stream_AddRef(s); - rdp = context->rdp; + rdpRdp* rdp = context->rdp; if (!rdp) goto fail; @@ -1158,8 +1166,8 @@ static int transport_default_write(rdpTransport* transport, wStream* s) if (!transport->frontBio) goto out_cleanup; - length = Stream_GetPosition(s); - writtenlength = length; + size_t length = Stream_GetPosition(s); + size_t writtenlength = length; Stream_SetPosition(s, 0); if (length > 0) @@ -1171,7 +1179,8 @@ static int transport_default_write(rdpTransport* transport, wStream* s) while (length > 0) { ERR_clear_error(); - status = BIO_write(transport->frontBio, Stream_ConstPointer(s), length); + const int towrite = (length > INT32_MAX) ? INT32_MAX : (int)length; + status = BIO_write(transport->frontBio, Stream_ConstPointer(s), towrite); if (status <= 0) { @@ -1223,8 +1232,8 @@ static int transport_default_write(rdpTransport* transport, wStream* s) } } - length -= status; - Stream_Seek(s, status); + length -= (size_t)status; + Stream_Seek(s, (size_t)status); } transport->written += writtenlength; @@ -1374,7 +1383,7 @@ BOOL transport_is_write_blocked(rdpTransport* transport) { WINPR_ASSERT(transport); WINPR_ASSERT(transport->frontBio); - return BIO_write_blocked(transport->frontBio); + return BIO_write_blocked(transport->frontBio) != 0; } int transport_drain_output_buffer(rdpTransport* transport) @@ -1388,7 +1397,8 @@ int transport_drain_output_buffer(rdpTransport* transport) if (BIO_flush(transport->frontBio) < 1) return -1; - status |= BIO_write_blocked(transport->frontBio); + const long rc = BIO_write_blocked(transport->frontBio); + status = (rc != 0); } return status;