From 6327b774611c9910ea0b12bec92c69dc4ff4897b Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 17:25:50 +0200 Subject: [PATCH 1/7] [winpr,crypto] fix type warnings in test case --- winpr/libwinpr/crypto/test/TestCryptoCipher.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/winpr/libwinpr/crypto/test/TestCryptoCipher.c b/winpr/libwinpr/crypto/test/TestCryptoCipher.c index a1977a0c5..da58eec6e 100644 --- a/winpr/libwinpr/crypto/test/TestCryptoCipher.c +++ b/winpr/libwinpr/crypto/test/TestCryptoCipher.c @@ -106,28 +106,25 @@ out: return result; } -static const BYTE* TEST_RC4_KEY = (BYTE*)"Key"; -static const char* TEST_RC4_PLAINTEXT = "Plaintext"; -static const BYTE* TEST_RC4_CIPHERTEXT = (BYTE*)"\xBB\xF3\x16\xE8\xD9\x40\xAF\x0A\xD3"; +static const char TEST_RC4_KEY[] = "Key"; +static const char TEST_RC4_PLAINTEXT[] = "Plaintext"; +static const char TEST_RC4_CIPHERTEXT[] = "\xBB\xF3\x16\xE8\xD9\x40\xAF\x0A\xD3"; static BOOL test_crypto_cipher_rc4(void) { - size_t len = 0; BOOL rc = FALSE; - BYTE* text = NULL; WINPR_RC4_CTX* ctx = NULL; - len = strnlen(TEST_RC4_PLAINTEXT, sizeof(TEST_RC4_PLAINTEXT)); - - if (!(text = (BYTE*)calloc(1, len))) + const size_t len = strnlen(TEST_RC4_PLAINTEXT, sizeof(TEST_RC4_PLAINTEXT)); + BYTE* text = (BYTE*)calloc(1, len); + if (!text) { (void)fprintf(stderr, "%s: failed to allocate text buffer (len=%" PRIuz ")\n", __func__, len); goto out; } - if ((ctx = winpr_RC4_New(TEST_RC4_KEY, - strnlen((const char*)TEST_RC4_KEY, sizeof(TEST_RC4_KEY)))) == NULL) + if ((ctx = winpr_RC4_New(TEST_RC4_KEY, strnlen(TEST_RC4_KEY, sizeof(TEST_RC4_KEY)))) == NULL) { (void)fprintf(stderr, "%s: winpr_RC4_New failed\n", __func__); goto out; From 2bcf2c50eb5aa04c86a9f2f6fe4f614bcfd88507 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 17:24:07 +0200 Subject: [PATCH 2/7] [channels,rdpear] fix krb5 inclusion * do not expose the krb5 include path in interface library, it is private to the object library * fix include krb5.h instead of krb5/krb5.h --- channels/rdpear/CMakeLists.txt | 2 +- channels/rdpear/client/rdpear_main.c | 2 +- channels/rdpear/common/CMakeLists.txt | 21 ++++++++++++++++++--- cmake/FindKRB5.cmake | 16 ++++++++-------- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/channels/rdpear/CMakeLists.txt b/channels/rdpear/CMakeLists.txt index 31f98cca8..8d0766200 100644 --- a/channels/rdpear/CMakeLists.txt +++ b/channels/rdpear/CMakeLists.txt @@ -35,4 +35,4 @@ if (NOT IOS AND NOT WIN32 AND NOT ANDROID) # add_channel_server(${MODULE_PREFIX} ${CHANNEL_NAME}) #endif() endif() -endif() \ No newline at end of file +endif() diff --git a/channels/rdpear/client/rdpear_main.c b/channels/rdpear/client/rdpear_main.c index a117740e6..97c45037a 100644 --- a/channels/rdpear/client/rdpear_main.c +++ b/channels/rdpear/client/rdpear_main.c @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include +#include #include #include diff --git a/channels/rdpear/common/CMakeLists.txt b/channels/rdpear/common/CMakeLists.txt index 448fa9364..c6915d3d5 100644 --- a/channels/rdpear/common/CMakeLists.txt +++ b/channels/rdpear/common/CMakeLists.txt @@ -26,10 +26,25 @@ add_library(rdpear-common-obj OBJECT ) target_include_directories(rdpear-common - INTERFACE ${KRB5_INCLUDE_DIRS}) -target_include_directories(rdpear-common-obj - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ${KRB5_INCLUDE_DIRS}) + SYSTEM + INTERFACE ${KRB5_INCLUDE_DIRS} +) +target_compile_options(rdpear-common + INTERFACE + ${KRB5_CFLAGS} +) +target_link_options(rdpear-common + INTERFACE + ${KRB5_LDFLAGS} +) +target_include_directories(rdpear-common-obj + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} +) +target_include_directories(rdpear-common-obj + SYSTEM + PRIVATE ${KRB5_INCLUDE_DIRS} +) target_link_directories(rdpear-common INTERFACE ${KRB5_LIBRARY_DIRS}) target_link_libraries(rdpear-common INTERFACE ${KRB5_LIBRARIES} diff --git a/cmake/FindKRB5.cmake b/cmake/FindKRB5.cmake index 5d5cde1cd..a9365e672 100644 --- a/cmake/FindKRB5.cmake +++ b/cmake/FindKRB5.cmake @@ -201,14 +201,14 @@ else() GET_KRB5_BY_CONFIG("${KRB5_ROOT_CONFIG}") endif() -#message("using KRB5_FOUND ${KRB5_FOUND} ") -#message("using KRB5_VERSION ${KRB5_VERSION} ") -#message("using KRB5_FLAVOUR ${KRB5_FLAVOUR} ") -#message("using KRB5_CFLAGS ${KRB5_CFLAGS} ") -#message("using KRB5_LDFLAGS ${KRB5_LDFLAGS} ") -#message("using KRB5_INCLUDEDIR ${KRB5_INCLUDEDIR} ") -#message("using KRB5_INCLUDE_DIRS ${KRB5_INCLUDEDIR} ") -#message("using KRB5_LIBRARIES ${KRB5_LIBRARIES} ") +message("using KRB5_FOUND ${KRB5_FOUND} ") +message("using KRB5_VERSION ${KRB5_VERSION} ") +message("using KRB5_FLAVOUR ${KRB5_FLAVOUR} ") +message("using KRB5_CFLAGS ${KRB5_CFLAGS} ") +message("using KRB5_LDFLAGS ${KRB5_LDFLAGS} ") +message("using KRB5_INCLUDEDIR ${KRB5_INCLUDEDIR} ") +message("using KRB5_INCLUDE_DIRS ${KRB5_INCLUDE_DIRS} ") +message("using KRB5_LIBRARIES ${KRB5_LIBRARIES} ") include(FindPackageHandleStandardArgs) From 3460f64f579aad365a44632068464b85573d3f3c Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 20:34:59 +0200 Subject: [PATCH 3/7] [client,common] fix possible NULL dereference --- client/common/cmdline.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/common/cmdline.c b/client/common/cmdline.c index b75f534e3..d50cff5b1 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -434,15 +434,16 @@ static size_t print_description(const char* text, size_t start_offset, size_t cu char* str = _strdup(text); char* cur = str; - do - { + while (cur != NULL) cur = print_token(cur, start_offset, ¤t, limit, " ", "\r\n"); - } while (cur != NULL); free(str); const int rc = printf("\n"); if (rc >= 0) + { + WINPR_ASSERT(SIZE_MAX - rc >= current); current += (size_t)rc; + } return current; } From 9cfd748b639eea4930a46b6f2a05046f3ff593a4 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 20:35:28 +0200 Subject: [PATCH 4/7] [core,nla] nla_read_TSRemoteGuardPackageCred * fix maybe uninitialized arguments * fix return in case of invalid packet --- libfreerdp/core/nla.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/libfreerdp/core/nla.c b/libfreerdp/core/nla.c index 2508bd2b0..4c471d36f 100644 --- a/libfreerdp/core/nla.c +++ b/libfreerdp/core/nla.c @@ -1198,6 +1198,7 @@ static BOOL nla_read_KERB_TICKET_LOGON(rdpNla* nla, wStream* s, KERB_TICKET_LOGO /** @brief kind of RCG credentials */ typedef enum { + RCG_TYPE_NONE, RCG_TYPE_KERB, RCG_TYPE_NTLM } RemoteGuardPackageCredType; @@ -1211,17 +1212,24 @@ static BOOL nla_read_TSRemoteGuardPackageCred(rdpNla* nla, WinPrAsn1Decoder* dec BOOL error = FALSE; char packageNameStr[100] = { 0 }; + WINPR_ASSERT(nla); + WINPR_ASSERT(dec); + WINPR_ASSERT(credsType); + WINPR_ASSERT(payload); + + *credsType = RCG_TYPE_NONE; + /* packageName [0] OCTET STRING */ if (!WinPrAsn1DecReadContextualOctetString(dec, 0, &error, &packageName, FALSE) || error) - return TRUE; + return FALSE; ConvertMszWCharNToUtf8((WCHAR*)packageName.data, packageName.len / sizeof(WCHAR), - packageNameStr, 100); + packageNameStr, sizeof(packageNameStr)); WLog_DBG(TAG, "TSRemoteGuardPackageCred(%s)", packageNameStr); /* credBuffer [1] OCTET STRING, */ if (!WinPrAsn1DecReadContextualOctetString(dec, 1, &error, &credBuffer, FALSE) || error) - return TRUE; + return FALSE; if (_stricmp(packageNameStr, "Kerberos") == 0) { @@ -1345,7 +1353,7 @@ static BOOL nla_read_ts_credentials(rdpNla* nla, SecBuffer* data) if (!WinPrAsn1DecReadContextualSequence(&dec2, 0, &error, &logonCredsSeq) || error) return FALSE; - RemoteGuardPackageCredType logonCredsType = { 0 }; + RemoteGuardPackageCredType logonCredsType = RCG_TYPE_NONE; wStream logonPayload = { 0 }; if (!nla_read_TSRemoteGuardPackageCred(nla, &logonCredsSeq, &logonCredsType, &logonPayload)) From 65de25205b4a83c791bbc1fc13774b4b6cdf88c7 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 20:42:07 +0200 Subject: [PATCH 5/7] [winpr,env] add missing WINPR_ATTR_MALLOC --- winpr/include/winpr/environment.h | 4 ++++ winpr/include/winpr/path.h | 10 ++++++++++ winpr/libwinpr/environment/environment.c | 2 ++ 3 files changed, 16 insertions(+) diff --git a/winpr/include/winpr/environment.h b/winpr/include/winpr/environment.h index c963f8c9c..cd5c1e66e 100644 --- a/winpr/include/winpr/environment.h +++ b/winpr/include/winpr/environment.h @@ -129,15 +129,19 @@ extern "C" { #endif + WINPR_ATTR_MALLOC(free, 1) WINPR_API LPCH MergeEnvironmentStrings(PCSTR original, PCSTR merge); WINPR_API DWORD GetEnvironmentVariableEBA(LPCSTR envBlock, LPCSTR lpName, LPSTR lpBuffer, DWORD nSize); WINPR_API BOOL SetEnvironmentVariableEBA(LPSTR* envBlock, LPCSTR lpName, LPCSTR lpValue); + WINPR_ATTR_MALLOC(free, 1) WINPR_API char** EnvironmentBlockToEnvpA(LPCH lpszEnvironmentBlock); WINPR_API DWORD GetEnvironmentVariableX(const char* lpName, char* lpBuffer, DWORD nSize); + + WINPR_ATTR_MALLOC(free, 1) WINPR_API char* GetEnvAlloc(LPCSTR lpName); #ifdef __cplusplus diff --git a/winpr/include/winpr/path.h b/winpr/include/winpr/path.h index 4554f49eb..975bfcf44 100644 --- a/winpr/include/winpr/path.h +++ b/winpr/include/winpr/path.h @@ -306,10 +306,20 @@ extern "C" #endif WINPR_API const char* GetKnownPathIdString(int id); + + WINPR_ATTR_MALLOC(free, 1) WINPR_API char* GetKnownPath(int id); + + WINPR_ATTR_MALLOC(free, 1) WINPR_API char* GetKnownSubPath(int id, const char* path); + + WINPR_ATTR_MALLOC(free, 1) WINPR_API char* GetEnvironmentPath(char* name); + + WINPR_ATTR_MALLOC(free, 1) WINPR_API char* GetEnvironmentSubPath(char* name, const char* path); + + WINPR_ATTR_MALLOC(free, 1) WINPR_API char* GetCombinedPath(const char* basePath, const char* subPath); WINPR_API BOOL PathMakePathA(LPCSTR path, LPSECURITY_ATTRIBUTES lpAttributes); diff --git a/winpr/libwinpr/environment/environment.c b/winpr/libwinpr/environment/environment.c index 191027c48..600b15f20 100644 --- a/winpr/libwinpr/environment/environment.c +++ b/winpr/libwinpr/environment/environment.c @@ -301,6 +301,8 @@ BOOL FreeEnvironmentStringsA(LPCH lpszEnvironmentBlock) BOOL FreeEnvironmentStringsW(LPWCH lpszEnvironmentBlock) { + free(lpszEnvironmentBlock); + return TRUE; } From a1cef8dd85f268a7bd0df9e1406af4ae4dba3762 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 20:55:48 +0200 Subject: [PATCH 6/7] [warnings] silence and fix unused results --- client/common/file.c | 4 ++-- libfreerdp/codec/test/TestFreeRDPCodecProgressive.c | 4 ++-- libfreerdp/common/assistance.c | 4 ++-- libfreerdp/core/license.c | 6 ++++-- libfreerdp/core/multitransport.c | 2 +- libfreerdp/core/smartcardlogon.c | 2 +- libfreerdp/core/streamdump.c | 2 +- libfreerdp/core/tcp.c | 4 ++-- libfreerdp/emu/scard/smartcard_virtual_gids.c | 3 ++- libfreerdp/utils/passphrase.c | 4 ++-- libfreerdp/utils/pcap.c | 4 ++-- winpr/libwinpr/file/namedPipeClient.c | 2 +- winpr/libwinpr/registry/registry_reg.c | 6 ++++-- winpr/libwinpr/utils/sam.c | 6 ++++-- winpr/libwinpr/utils/wlog/UdpAppender.c | 13 ++++++++----- winpr/libwinpr/winsock/winsock.c | 4 ++-- 16 files changed, 40 insertions(+), 30 deletions(-) diff --git a/client/common/file.c b/client/common/file.c index b2f896ae3..fe97d0cf1 100644 --- a/client/common/file.c +++ b/client/common/file.c @@ -1021,9 +1021,9 @@ BOOL freerdp_client_parse_rdp_file_ex(rdpFile* file, const char* name, rdp_file_ return FALSE; } - _fseeki64(fp, 0, SEEK_END); + (void)_fseeki64(fp, 0, SEEK_END); file_size = _ftelli64(fp); - _fseeki64(fp, 0, SEEK_SET); + (void)_fseeki64(fp, 0, SEEK_SET); if (file_size < 1) { diff --git a/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c b/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c index b5191d01d..869ebaa37 100644 --- a/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c +++ b/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c @@ -286,12 +286,12 @@ static BYTE* test_progressive_load_file(const char* path, const char* file, size if (!fp) return NULL; - _fseeki64(fp, 0, SEEK_END); + (void)_fseeki64(fp, 0, SEEK_END); const INT64 pos = _ftelli64(fp); WINPR_ASSERT(pos >= 0); WINPR_ASSERT(pos <= SIZE_MAX); *size = (size_t)pos; - _fseeki64(fp, 0, SEEK_SET); + (void)_fseeki64(fp, 0, SEEK_SET); BYTE* buffer = (BYTE*)malloc(*size); if (!buffer) diff --git a/libfreerdp/common/assistance.c b/libfreerdp/common/assistance.c index 1e4122938..cb52beb63 100644 --- a/libfreerdp/common/assistance.c +++ b/libfreerdp/common/assistance.c @@ -1246,9 +1246,9 @@ int freerdp_assistance_parse_file(rdpAssistanceFile* file, const char* name, con return -1; } - _fseeki64(fp, 0, SEEK_END); + (void)_fseeki64(fp, 0, SEEK_END); fileSize.i64 = _ftelli64(fp); - _fseeki64(fp, 0, SEEK_SET); + (void)_fseeki64(fp, 0, SEEK_SET); if (fileSize.i64 < 1) { diff --git a/libfreerdp/core/license.c b/libfreerdp/core/license.c index 223c2648d..143494bd4 100644 --- a/libfreerdp/core/license.c +++ b/libfreerdp/core/license.c @@ -624,9 +624,11 @@ static BYTE* loadCalFile(const rdpSettings* settings, const char* hostname, size if (!fp) goto error_open; - _fseeki64(fp, 0, SEEK_END); + if (_fseeki64(fp, 0, SEEK_END) != 0) + goto error_malloc; length = _ftelli64(fp); - _fseeki64(fp, 0, SEEK_SET); + if (_fseeki64(fp, 0, SEEK_SET) != 0) + goto error_malloc; if (length < 0) goto error_malloc; diff --git a/libfreerdp/core/multitransport.c b/libfreerdp/core/multitransport.c index f37870f7c..ce29d9908 100644 --- a/libfreerdp/core/multitransport.c +++ b/libfreerdp/core/multitransport.c @@ -85,7 +85,7 @@ state_run_t multitransport_recv_request(rdpMultitransport* multi, wStream* s) WLog_WARN(TAG, "reserved is %" PRIu16 " instead of 0, skipping %" PRIuz "bytes of unknown data", reserved, Stream_GetRemainingLength(s)); - Stream_SafeSeek(s, Stream_GetRemainingLength(s)); + (void)Stream_SafeSeek(s, Stream_GetRemainingLength(s)); } WINPR_ASSERT(multi->MtRequest); diff --git a/libfreerdp/core/smartcardlogon.c b/libfreerdp/core/smartcardlogon.c index cbd36bc7e..95192324e 100644 --- a/libfreerdp/core/smartcardlogon.c +++ b/libfreerdp/core/smartcardlogon.c @@ -57,7 +57,7 @@ static void delete_file(char* path) int rs = _fseeki64(fp, 0, SEEK_END); if (rs == 0) size = _ftelli64(fp); - _fseeki64(fp, 0, SEEK_SET); + (void)_fseeki64(fp, 0, SEEK_SET); for (INT64 x = 0; x < size; x += sizeof(buffer)) { diff --git a/libfreerdp/core/streamdump.c b/libfreerdp/core/streamdump.c index abe3ff5ef..3f1068091 100644 --- a/libfreerdp/core/streamdump.c +++ b/libfreerdp/core/streamdump.c @@ -80,7 +80,7 @@ static return FALSE; if (pOffset) - _fseeki64(fp, *pOffset, SEEK_SET); + (void)_fseeki64(fp, *pOffset, SEEK_SET); r = fread(&ts, 1, sizeof(ts), fp); if (r != sizeof(ts)) diff --git a/libfreerdp/core/tcp.c b/libfreerdp/core/tcp.c index da6731c63..beaf6c584 100644 --- a/libfreerdp/core/tcp.c +++ b/libfreerdp/core/tcp.c @@ -221,9 +221,9 @@ static long transport_bio_simple_ctrl(BIO* bio, int cmd, long arg1, void* arg2) return 0; if (arg1) - fcntl((int)ptr->socket, F_SETFL, flags | O_NONBLOCK); + (void)fcntl((int)ptr->socket, F_SETFL, flags | O_NONBLOCK); else - fcntl((int)ptr->socket, F_SETFL, flags & ~(O_NONBLOCK)); + (void)fcntl((int)ptr->socket, F_SETFL, flags & ~(O_NONBLOCK)); #else /* the internal socket is always non-blocking */ diff --git a/libfreerdp/emu/scard/smartcard_virtual_gids.c b/libfreerdp/emu/scard/smartcard_virtual_gids.c index 751de34d3..9e0e8d53f 100644 --- a/libfreerdp/emu/scard/smartcard_virtual_gids.c +++ b/libfreerdp/emu/scard/smartcard_virtual_gids.c @@ -401,7 +401,8 @@ static BOOL vgids_ef_read_do(vgidsEF* ef, UINT16 doID, BYTE** data, DWORD* dataS else { /* Skip DO */ - Stream_SafeSeek(ef->data, doSize); + if (!Stream_SafeSeek(ef->data, doSize)) + return FALSE; } } diff --git a/libfreerdp/utils/passphrase.c b/libfreerdp/utils/passphrase.c index ac10bdcca..be7934eae 100644 --- a/libfreerdp/utils/passphrase.c +++ b/libfreerdp/utils/passphrase.c @@ -252,7 +252,7 @@ int freerdp_interruptible_getc(rdpContext* context, FILE* f) const int fd = fileno(f); const int orig = fcntl(fd, F_GETFL); - fcntl(fd, F_SETFL, orig | O_NONBLOCK); + (void)fcntl(fd, F_SETFL, orig | O_NONBLOCK); do { const int res = wait_for_fd(fd, 10); @@ -266,7 +266,7 @@ int freerdp_interruptible_getc(rdpContext* context, FILE* f) } } while (!freerdp_shall_disconnect_context(context)); - fcntl(fd, F_SETFL, orig); + (void)fcntl(fd, F_SETFL, orig); return rc; } diff --git a/libfreerdp/utils/pcap.c b/libfreerdp/utils/pcap.c index 538e1325e..9dacbd09c 100644 --- a/libfreerdp/utils/pcap.c +++ b/libfreerdp/utils/pcap.c @@ -225,9 +225,9 @@ rdpPcap* pcap_open(const char* name, BOOL write) } else { - _fseeki64(pcap->fp, 0, SEEK_END); + (void)_fseeki64(pcap->fp, 0, SEEK_END); pcap->file_size = _ftelli64(pcap->fp); - _fseeki64(pcap->fp, 0, SEEK_SET); + (void)_fseeki64(pcap->fp, 0, SEEK_SET); if (!pcap_read_header(pcap, &pcap->header)) goto fail; } diff --git a/winpr/libwinpr/file/namedPipeClient.c b/winpr/libwinpr/file/namedPipeClient.c index 1cd1796e7..f7102700f 100644 --- a/winpr/libwinpr/file/namedPipeClient.c +++ b/winpr/libwinpr/file/namedPipeClient.c @@ -217,7 +217,7 @@ static HANDLE NamedPipeClientCreateFileA(LPCSTR lpFileName, DWORD dwDesiredAcces int flags = fcntl(pNamedPipe->clientfd, F_GETFL); if (flags != -1) - fcntl(pNamedPipe->clientfd, F_SETFL, flags | O_NONBLOCK); + (void)fcntl(pNamedPipe->clientfd, F_SETFL, flags | O_NONBLOCK); #endif } diff --git a/winpr/libwinpr/registry/registry_reg.c b/winpr/libwinpr/registry/registry_reg.c index e340896d4..835944d12 100644 --- a/winpr/libwinpr/registry/registry_reg.c +++ b/winpr/libwinpr/registry/registry_reg.c @@ -95,9 +95,11 @@ static BOOL reg_load_start(Reg* reg) WINPR_ASSERT(reg); WINPR_ASSERT(reg->fp); - _fseeki64(reg->fp, 0, SEEK_END); + if (_fseeki64(reg->fp, 0, SEEK_END) != 0) + return FALSE; file_size = _ftelli64(reg->fp); - _fseeki64(reg->fp, 0, SEEK_SET); + if (_fseeki64(reg->fp, 0, SEEK_SET) != 0) + return FALSE; reg->line = NULL; reg->next_line = NULL; diff --git a/winpr/libwinpr/utils/sam.c b/winpr/libwinpr/utils/sam.c index e67efecea..f737dcd85 100644 --- a/winpr/libwinpr/utils/sam.c +++ b/winpr/libwinpr/utils/sam.c @@ -139,9 +139,11 @@ static BOOL SamLookupStart(WINPR_SAM* sam) if (!sam || !sam->fp) return FALSE; - _fseeki64(sam->fp, 0, SEEK_END); + if (_fseeki64(sam->fp, 0, SEEK_END) != 0) + return FALSE; fileSize = _ftelli64(sam->fp); - _fseeki64(sam->fp, 0, SEEK_SET); + if (_fseeki64(sam->fp, 0, SEEK_SET) != 0) + return FALSE; if (fileSize < 1) return FALSE; diff --git a/winpr/libwinpr/utils/wlog/UdpAppender.c b/winpr/libwinpr/utils/wlog/UdpAppender.c index 5909e0b3c..5b25926e5 100644 --- a/winpr/libwinpr/utils/wlog/UdpAppender.c +++ b/winpr/libwinpr/utils/wlog/UdpAppender.c @@ -99,11 +99,14 @@ static BOOL WLog_UdpAppender_WriteMessage(wLog* log, wLogAppender* appender, wLo udpAppender = (wLogUdpAppender*)appender; message->PrefixString = prefix; WLog_Layout_GetMessagePrefix(log, appender->Layout, message); - _sendto(udpAppender->sock, message->PrefixString, (int)strnlen(message->PrefixString, INT_MAX), - 0, &udpAppender->targetAddr, udpAppender->targetAddrLen); - _sendto(udpAppender->sock, message->TextString, (int)strnlen(message->TextString, INT_MAX), 0, - &udpAppender->targetAddr, udpAppender->targetAddrLen); - _sendto(udpAppender->sock, "\n", 1, 0, &udpAppender->targetAddr, udpAppender->targetAddrLen); + (void)_sendto(udpAppender->sock, message->PrefixString, + (int)strnlen(message->PrefixString, INT_MAX), 0, &udpAppender->targetAddr, + udpAppender->targetAddrLen); + (void)_sendto(udpAppender->sock, message->TextString, + (int)strnlen(message->TextString, INT_MAX), 0, &udpAppender->targetAddr, + udpAppender->targetAddrLen); + (void)_sendto(udpAppender->sock, "\n", 1, 0, &udpAppender->targetAddr, + udpAppender->targetAddrLen); return TRUE; } diff --git a/winpr/libwinpr/winsock/winsock.c b/winpr/libwinpr/winsock/winsock.c index f36a88011..bb2b60c6c 100644 --- a/winpr/libwinpr/winsock/winsock.c +++ b/winpr/libwinpr/winsock/winsock.c @@ -1060,9 +1060,9 @@ int _ioctlsocket(SOCKET s, long cmd, u_long* argp) return SOCKET_ERROR; if (*argp) - fcntl(fd, F_SETFL, flags | O_NONBLOCK); + (void)fcntl(fd, F_SETFL, flags | O_NONBLOCK); else - fcntl(fd, F_SETFL, flags & ~(O_NONBLOCK)); + (void)fcntl(fd, F_SETFL, flags & ~(O_NONBLOCK)); } return 0; From 8b6091a0078b4d285d02b4310d6de5a0f1cbd97c Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 14 Sep 2024 21:07:03 +0200 Subject: [PATCH 7/7] [winpr,wtsapi] improve API usage * Mark WTSVirtualChannelOpen and WTSVirtualChannelOpenEx with WINPR_ATTR_MALLOC to enforce compiler checks for resource cleanup * Fix unused result warnings, use the result or cast to (void) where not requierd --- channels/ainput/server/ainput_main.c | 4 ++-- channels/audin/server/audin.c | 4 ++-- channels/cliprdr/server/cliprdr_main.c | 2 +- channels/disp/server/disp_main.c | 4 ++-- channels/echo/server/echo_main.c | 11 ++++++++--- channels/encomsp/server/encomsp_main.c | 9 +++++++-- channels/location/server/location_main.c | 4 ++-- channels/rail/server/rail_main.c | 4 ++-- channels/rdpdr/server/rdpdr_main.c | 2 +- .../rdpecam/server/camera_device_enumerator_main.c | 4 ++-- channels/rdpecam/server/camera_device_main.c | 4 ++-- channels/rdpei/server/rdpei_main.c | 4 ++-- channels/rdpemsc/server/mouse_cursor_main.c | 4 ++-- channels/rdpgfx/server/rdpgfx_main.c | 2 +- channels/rdpsnd/server/rdpsnd_main.c | 4 ++-- channels/remdesk/server/remdesk_main.c | 2 +- channels/telemetry/server/telemetry_main.c | 4 ++-- libfreerdp/core/server.c | 2 +- server/Sample/sfreerdp.c | 14 +++++++++----- winpr/include/winpr/wtsapi.h | 6 ++++-- 20 files changed, 55 insertions(+), 39 deletions(-) diff --git a/channels/ainput/server/ainput_main.c b/channels/ainput/server/ainput_main.c index 6bb465025..81a9b5113 100644 --- a/channels/ainput/server/ainput_main.c +++ b/channels/ainput/server/ainput_main.c @@ -292,7 +292,7 @@ static DWORD WINAPI ainput_server_thread_func(LPVOID arg) } } - WTSVirtualChannelClose(ainput->ainput_channel); + (void)WTSVirtualChannelClose(ainput->ainput_channel); ainput->ainput_channel = NULL; if (error && ainput->context.rdpcontext) @@ -369,7 +369,7 @@ static UINT ainput_server_close(ainput_server_context* context) { if (ainput->state != AINPUT_INITIAL) { - WTSVirtualChannelClose(ainput->ainput_channel); + (void)WTSVirtualChannelClose(ainput->ainput_channel); ainput->ainput_channel = NULL; ainput->state = AINPUT_INITIAL; } diff --git a/channels/audin/server/audin.c b/channels/audin/server/audin.c index d67937ab7..ac1e53626 100644 --- a/channels/audin/server/audin.c +++ b/channels/audin/server/audin.c @@ -430,7 +430,7 @@ static DWORD WINAPI audin_server_thread_func(LPVOID arg) out_capacity: Stream_Free(s, TRUE); out: - WTSVirtualChannelClose(audin->audin_channel); + (void)WTSVirtualChannelClose(audin->audin_channel); audin->audin_channel = NULL; if (error && audin->context.rdpcontext) @@ -533,7 +533,7 @@ static BOOL audin_server_close(audin_server_context* context) if (audin->audin_channel) { - WTSVirtualChannelClose(audin->audin_channel); + (void)WTSVirtualChannelClose(audin->audin_channel); audin->audin_channel = NULL; } diff --git a/channels/cliprdr/server/cliprdr_main.c b/channels/cliprdr/server/cliprdr_main.c index 44dc68e07..c8f961d52 100644 --- a/channels/cliprdr/server/cliprdr_main.c +++ b/channels/cliprdr/server/cliprdr_main.c @@ -1355,7 +1355,7 @@ static UINT cliprdr_server_close(CliprdrServerContext* context) if (cliprdr->ChannelHandle) { - WTSVirtualChannelClose(cliprdr->ChannelHandle); + (void)WTSVirtualChannelClose(cliprdr->ChannelHandle); cliprdr->ChannelHandle = NULL; } diff --git a/channels/disp/server/disp_main.c b/channels/disp/server/disp_main.c index 931d8a317..b7bbde684 100644 --- a/channels/disp/server/disp_main.c +++ b/channels/disp/server/disp_main.c @@ -474,7 +474,7 @@ static UINT disp_server_open(DispServerContext* context) return CHANNEL_RC_OK; out_close: - WTSVirtualChannelClose(priv->disp_channel); + (void)WTSVirtualChannelClose(priv->disp_channel); priv->disp_channel = NULL; priv->channelEvent = NULL; return rc; @@ -567,7 +567,7 @@ static UINT disp_server_close(DispServerContext* context) if (priv->disp_channel) { - WTSVirtualChannelClose(priv->disp_channel); + (void)WTSVirtualChannelClose(priv->disp_channel); priv->disp_channel = NULL; } diff --git a/channels/echo/server/echo_main.c b/channels/echo/server/echo_main.c index 9c165261c..c31c55eae 100644 --- a/channels/echo/server/echo_main.c +++ b/channels/echo/server/echo_main.c @@ -219,7 +219,7 @@ static DWORD WINAPI echo_server_thread_func(LPVOID arg) if (!s) { WLog_ERR(TAG, "Stream_New failed!"); - WTSVirtualChannelClose(echo->echo_channel); + (void)WTSVirtualChannelClose(echo->echo_channel); ExitThread(ERROR_NOT_ENOUGH_MEMORY); return ERROR_NOT_ENOUGH_MEMORY; } @@ -239,7 +239,12 @@ static DWORD WINAPI echo_server_thread_func(LPVOID arg) break; Stream_SetPosition(s, 0); - WTSVirtualChannelRead(echo->echo_channel, 0, NULL, 0, &BytesReturned); + if (!WTSVirtualChannelRead(echo->echo_channel, 0, NULL, 0, &BytesReturned)) + { + WLog_ERR(TAG, "WTSVirtualChannelRead failed!"); + error = ERROR_INTERNAL_ERROR; + break; + } if (BytesReturned < 1) continue; @@ -270,7 +275,7 @@ static DWORD WINAPI echo_server_thread_func(LPVOID arg) } Stream_Free(s, TRUE); - WTSVirtualChannelClose(echo->echo_channel); + (void)WTSVirtualChannelClose(echo->echo_channel); echo->echo_channel = NULL; out: diff --git a/channels/encomsp/server/encomsp_main.c b/channels/encomsp/server/encomsp_main.c index f6390dfda..a7bf075b4 100644 --- a/channels/encomsp/server/encomsp_main.c +++ b/channels/encomsp/server/encomsp_main.c @@ -232,7 +232,12 @@ static DWORD WINAPI encomsp_server_thread(LPVOID arg) break; } - WTSVirtualChannelRead(context->priv->ChannelHandle, 0, NULL, 0, &BytesReturned); + if (!WTSVirtualChannelRead(context->priv->ChannelHandle, 0, NULL, 0, &BytesReturned)) + { + WLog_ERR(TAG, "WTSVirtualChannelRead failed!"); + error = ERROR_INTERNAL_ERROR; + break; + } if (BytesReturned < 1) continue; @@ -364,7 +369,7 @@ void encomsp_server_context_free(EncomspServerContext* context) if (context) { if (context->priv->ChannelHandle != INVALID_HANDLE_VALUE) - WTSVirtualChannelClose(context->priv->ChannelHandle); + (void)WTSVirtualChannelClose(context->priv->ChannelHandle); free(context->priv); free(context); diff --git a/channels/location/server/location_main.c b/channels/location/server/location_main.c index bea88edee..549cfc019 100644 --- a/channels/location/server/location_main.c +++ b/channels/location/server/location_main.c @@ -426,7 +426,7 @@ static DWORD WINAPI location_server_thread_func(LPVOID arg) } } - WTSVirtualChannelClose(location->location_channel); + (void)WTSVirtualChannelClose(location->location_channel); location->location_channel = NULL; if (error && location->context.rdpcontext) @@ -493,7 +493,7 @@ static UINT location_server_close(LocationServerContext* context) { if (location->state != LOCATION_INITIAL) { - WTSVirtualChannelClose(location->location_channel); + (void)WTSVirtualChannelClose(location->location_channel); location->location_channel = NULL; location->state = LOCATION_INITIAL; } diff --git a/channels/rail/server/rail_main.c b/channels/rail/server/rail_main.c index 8161d1751..bc569bba7 100644 --- a/channels/rail/server/rail_main.c +++ b/channels/rail/server/rail_main.c @@ -1462,7 +1462,7 @@ out_stop_event: CloseHandle(context->priv->stopEvent); context->priv->stopEvent = NULL; out_close: - WTSVirtualChannelClose(context->priv->rail_channel); + (void)WTSVirtualChannelClose(context->priv->rail_channel); context->priv->rail_channel = NULL; return error; } @@ -1489,7 +1489,7 @@ static BOOL rail_server_stop(RailServerContext* context) if (priv->rail_channel) { - WTSVirtualChannelClose(priv->rail_channel); + (void)WTSVirtualChannelClose(priv->rail_channel); priv->rail_channel = NULL; } diff --git a/channels/rdpdr/server/rdpdr_main.c b/channels/rdpdr/server/rdpdr_main.c index 57c1da52d..70c13fb1c 100644 --- a/channels/rdpdr/server/rdpdr_main.c +++ b/channels/rdpdr/server/rdpdr_main.c @@ -2186,7 +2186,7 @@ static UINT rdpdr_server_stop(RdpdrServerContext* context) if (context->priv->ChannelHandle) { - WTSVirtualChannelClose(context->priv->ChannelHandle); + (void)WTSVirtualChannelClose(context->priv->ChannelHandle); context->priv->ChannelHandle = NULL; } return CHANNEL_RC_OK; diff --git a/channels/rdpecam/server/camera_device_enumerator_main.c b/channels/rdpecam/server/camera_device_enumerator_main.c index 27523b0f1..195ec8454 100644 --- a/channels/rdpecam/server/camera_device_enumerator_main.c +++ b/channels/rdpecam/server/camera_device_enumerator_main.c @@ -418,7 +418,7 @@ static DWORD WINAPI enumerator_server_thread_func(LPVOID arg) } } - WTSVirtualChannelClose(enumerator->enumerator_channel); + (void)WTSVirtualChannelClose(enumerator->enumerator_channel); enumerator->enumerator_channel = NULL; if (error && enumerator->context.rdpcontext) @@ -486,7 +486,7 @@ static UINT enumerator_server_close(CamDevEnumServerContext* context) { if (enumerator->state != ENUMERATOR_INITIAL) { - WTSVirtualChannelClose(enumerator->enumerator_channel); + (void)WTSVirtualChannelClose(enumerator->enumerator_channel); enumerator->enumerator_channel = NULL; enumerator->state = ENUMERATOR_INITIAL; } diff --git a/channels/rdpecam/server/camera_device_main.c b/channels/rdpecam/server/camera_device_main.c index ce0774c51..0d3acdd2e 100644 --- a/channels/rdpecam/server/camera_device_main.c +++ b/channels/rdpecam/server/camera_device_main.c @@ -571,7 +571,7 @@ static DWORD WINAPI device_server_thread_func(LPVOID arg) } } - WTSVirtualChannelClose(device->device_channel); + (void)WTSVirtualChannelClose(device->device_channel); device->device_channel = NULL; if (error && device->context.rdpcontext) @@ -638,7 +638,7 @@ static UINT device_server_close(CameraDeviceServerContext* context) { if (device->state != CAMERA_DEVICE_INITIAL) { - WTSVirtualChannelClose(device->device_channel); + (void)WTSVirtualChannelClose(device->device_channel); device->device_channel = NULL; device->state = CAMERA_DEVICE_INITIAL; } diff --git a/channels/rdpei/server/rdpei_main.c b/channels/rdpei/server/rdpei_main.c index c87a045db..c4d713a5e 100644 --- a/channels/rdpei/server/rdpei_main.c +++ b/channels/rdpei/server/rdpei_main.c @@ -136,7 +136,7 @@ UINT rdpei_server_init(RdpeiServerContext* context) return CHANNEL_RC_OK; out_close: - WTSVirtualChannelClose(priv->channelHandle); + (void)WTSVirtualChannelClose(priv->channelHandle); return CHANNEL_RC_INITIALIZATION_ERROR; } @@ -161,7 +161,7 @@ void rdpei_server_context_free(RdpeiServerContext* context) if (priv) { if (priv->channelHandle != INVALID_HANDLE_VALUE) - WTSVirtualChannelClose(priv->channelHandle); + (void)WTSVirtualChannelClose(priv->channelHandle); Stream_Free(priv->inputStream, TRUE); } free(priv); diff --git a/channels/rdpemsc/server/mouse_cursor_main.c b/channels/rdpemsc/server/mouse_cursor_main.c index d7d2f789b..1e5225dfb 100644 --- a/channels/rdpemsc/server/mouse_cursor_main.c +++ b/channels/rdpemsc/server/mouse_cursor_main.c @@ -375,7 +375,7 @@ static DWORD WINAPI mouse_cursor_server_thread_func(LPVOID arg) } } - WTSVirtualChannelClose(mouse_cursor->mouse_cursor_channel); + (void)WTSVirtualChannelClose(mouse_cursor->mouse_cursor_channel); mouse_cursor->mouse_cursor_channel = NULL; if (error && mouse_cursor->context.rdpcontext) @@ -443,7 +443,7 @@ static UINT mouse_cursor_server_close(MouseCursorServerContext* context) { if (mouse_cursor->state != MOUSE_CURSOR_INITIAL) { - WTSVirtualChannelClose(mouse_cursor->mouse_cursor_channel); + (void)WTSVirtualChannelClose(mouse_cursor->mouse_cursor_channel); mouse_cursor->mouse_cursor_channel = NULL; mouse_cursor->state = MOUSE_CURSOR_INITIAL; } diff --git a/channels/rdpgfx/server/rdpgfx_main.c b/channels/rdpgfx/server/rdpgfx_main.c index 534c5577c..3b21d03cb 100644 --- a/channels/rdpgfx/server/rdpgfx_main.c +++ b/channels/rdpgfx/server/rdpgfx_main.c @@ -1649,7 +1649,7 @@ BOOL rdpgfx_server_close(RdpgfxServerContext* context) if (priv->rdpgfx_channel) { - WTSVirtualChannelClose(priv->rdpgfx_channel); + (void)WTSVirtualChannelClose(priv->rdpgfx_channel); priv->rdpgfx_channel = NULL; } diff --git a/channels/rdpsnd/server/rdpsnd_main.c b/channels/rdpsnd/server/rdpsnd_main.c index f1b8fcfbb..9d3e3e616 100644 --- a/channels/rdpsnd/server/rdpsnd_main.c +++ b/channels/rdpsnd/server/rdpsnd_main.c @@ -965,7 +965,7 @@ out_pdu: Stream_Free(context->priv->rdpsnd_pdu, TRUE); context->priv->rdpsnd_pdu = NULL; out_close: - WTSVirtualChannelClose(context->priv->ChannelHandle); + (void)WTSVirtualChannelClose(context->priv->ChannelHandle); context->priv->ChannelHandle = NULL; return error; } @@ -1015,7 +1015,7 @@ static UINT rdpsnd_server_stop(RdpsndServerContext* context) if (context->priv->ChannelHandle) { - WTSVirtualChannelClose(context->priv->ChannelHandle); + (void)WTSVirtualChannelClose(context->priv->ChannelHandle); context->priv->ChannelHandle = NULL; } diff --git a/channels/remdesk/server/remdesk_main.c b/channels/remdesk/server/remdesk_main.c index 5aed1e3dd..90f7e97ec 100644 --- a/channels/remdesk/server/remdesk_main.c +++ b/channels/remdesk/server/remdesk_main.c @@ -728,7 +728,7 @@ void remdesk_server_context_free(RemdeskServerContext* context) if (context) { if (context->priv->ChannelHandle != INVALID_HANDLE_VALUE) - WTSVirtualChannelClose(context->priv->ChannelHandle); + (void)WTSVirtualChannelClose(context->priv->ChannelHandle); free(context->priv); free(context); diff --git a/channels/telemetry/server/telemetry_main.c b/channels/telemetry/server/telemetry_main.c index 0d8e0476c..329fd7b55 100644 --- a/channels/telemetry/server/telemetry_main.c +++ b/channels/telemetry/server/telemetry_main.c @@ -295,7 +295,7 @@ static DWORD WINAPI telemetry_server_thread_func(LPVOID arg) } } - WTSVirtualChannelClose(telemetry->telemetry_channel); + (void)WTSVirtualChannelClose(telemetry->telemetry_channel); telemetry->telemetry_channel = NULL; if (error && telemetry->context.rdpcontext) @@ -362,7 +362,7 @@ static UINT telemetry_server_close(TelemetryServerContext* context) { if (telemetry->state != TELEMETRY_INITIAL) { - WTSVirtualChannelClose(telemetry->telemetry_channel); + (void)WTSVirtualChannelClose(telemetry->telemetry_channel); telemetry->telemetry_channel = NULL; telemetry->state = TELEMETRY_INITIAL; } diff --git a/libfreerdp/core/server.c b/libfreerdp/core/server.c index fe3c2cc5a..72cffdd72 100644 --- a/libfreerdp/core/server.c +++ b/libfreerdp/core/server.c @@ -1066,7 +1066,7 @@ VOID WINAPI FreeRDP_WTSCloseServer(HANDLE hServer) if (vcm->drdynvc_channel) { - WTSVirtualChannelClose(vcm->drdynvc_channel); + (void)WTSVirtualChannelClose(vcm->drdynvc_channel); vcm->drdynvc_channel = NULL; } diff --git a/server/Sample/sfreerdp.c b/server/Sample/sfreerdp.c index edf4cea0a..c873efcc5 100644 --- a/server/Sample/sfreerdp.c +++ b/server/Sample/sfreerdp.c @@ -93,7 +93,7 @@ static void test_peer_context_free(freerdp_peer* client, rdpContext* ctx) nsc_context_free(context->nsc_context); if (context->debug_channel) - WTSVirtualChannelClose(context->debug_channel); + (void)WTSVirtualChannelClose(context->debug_channel); sf_peer_audin_uninit(context); @@ -610,7 +610,6 @@ fail: static DWORD WINAPI tf_debug_channel_thread_func(LPVOID arg) { void* fd = NULL; - wStream* s = NULL; void* buffer = NULL; DWORD BytesReturned = 0; ULONG written = 0; @@ -627,8 +626,12 @@ static DWORD WINAPI tf_debug_channel_thread_func(LPVOID arg) return 0; } - s = Stream_New(NULL, 4096); - WTSVirtualChannelWrite(context->debug_channel, (PCHAR) "test1", 5, &written); + wStream* s = Stream_New(NULL, 4096); + if (!s) + goto fail; + + if (!WTSVirtualChannelWrite(context->debug_channel, (PCHAR) "test1", 5, &written)) + goto fail; while (1) { @@ -892,7 +895,8 @@ static BOOL tf_peer_keyboard_event(rdpInput* input, UINT16 flags, UINT8 code) if (tcontext->debug_channel) { ULONG written = 0; - WTSVirtualChannelWrite(tcontext->debug_channel, (PCHAR) "test2", 5, &written); + if (!WTSVirtualChannelWrite(tcontext->debug_channel, (PCHAR) "test2", 5, &written)) + return FALSE; } } else if (((flags & KBD_FLAGS_RELEASE) == 0) && code == RDP_SCANCODE_KEY_X) /* 'x' key */ diff --git a/winpr/include/winpr/wtsapi.h b/winpr/include/winpr/wtsapi.h index 9a0ee6ae6..6bb64fc6c 100644 --- a/winpr/include/winpr/wtsapi.h +++ b/winpr/include/winpr/wtsapi.h @@ -1074,14 +1074,16 @@ extern "C" WINPR_API BOOL WINAPI WTSWaitSystemEvent(HANDLE hServer, DWORD EventMask, DWORD* pEventFlags); + WINPR_API BOOL WINAPI WTSVirtualChannelClose(HANDLE hChannelHandle); + + WINPR_ATTR_MALLOC(WTSVirtualChannelClose, 1) WINPR_API HANDLE WINAPI WTSVirtualChannelOpen(HANDLE hServer, DWORD SessionId, LPSTR pVirtualName); + WINPR_ATTR_MALLOC(WTSVirtualChannelClose, 1) WINPR_API HANDLE WINAPI WTSVirtualChannelOpenEx(DWORD SessionId, LPSTR pVirtualName, DWORD flags); - WINPR_API BOOL WINAPI WTSVirtualChannelClose(HANDLE hChannelHandle); - WINPR_API BOOL WINAPI WTSVirtualChannelRead(HANDLE hChannelHandle, ULONG TimeOut, PCHAR Buffer, ULONG BufferSize, PULONG pBytesRead);