From eba2bf463c0d796060c01c30a6c2e6d3208752c6 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 08:35:15 +0100 Subject: [PATCH 1/6] [channels,rail] refactor server side functions * proper error handling * use [[nodiscard]] --- channels/rail/rail_common.c | 12 +- channels/rail/server/rail_main.c | 421 ++++++++++++++----------------- include/freerdp/server/rail.h | 1 - 3 files changed, 198 insertions(+), 236 deletions(-) diff --git a/channels/rail/rail_common.c b/channels/rail/rail_common.c index b5c796f1c..3ab9ac7c3 100644 --- a/channels/rail/rail_common.c +++ b/channels/rail/rail_common.c @@ -149,6 +149,9 @@ UINT rail_read_handshake_order(wStream* s, RAIL_HANDSHAKE_ORDER* handshake) void rail_write_handshake_order(wStream* s, const RAIL_HANDSHAKE_ORDER* handshake) { + WINPR_ASSERT(s); + WINPR_ASSERT(handshake); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 4)); Stream_Write_UINT32(s, handshake->buildNumber); /* buildNumber (4 bytes) */ } @@ -169,6 +172,10 @@ UINT rail_read_handshake_ex_order(wStream* s, RAIL_HANDSHAKE_EX_ORDER* handshake void rail_write_handshake_ex_order(wStream* s, const RAIL_HANDSHAKE_EX_ORDER* handshakeEx) { + WINPR_ASSERT(s); + WINPR_ASSERT(handshakeEx); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 8)); + Stream_Write_UINT32(s, handshakeEx->buildNumber); /* buildNumber (4 bytes) */ Stream_Write_UINT32(s, handshakeEx->railHandshakeFlags); /* railHandshakeFlags (4 bytes) */ } @@ -471,11 +478,6 @@ UINT rail_read_sysparam_order(wStream* s, RAIL_SYSPARAM_ORDER* sysparam, BOOL ex return error; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 err2or code - */ UINT rail_write_sysparam_order(wStream* s, const RAIL_SYSPARAM_ORDER* sysparam, BOOL extendedSpiSupported) { diff --git a/channels/rail/server/rail_main.c b/channels/rail/server/rail_main.c index b9208d0d3..e011a3ac8 100644 --- a/channels/rail/server/rail_main.c +++ b/channels/rail/server/rail_main.c @@ -38,15 +38,16 @@ * * @return 0 on success, otherwise a Win32 error code */ -static UINT rail_send(RailServerContext* context, wStream* s, ULONG length) +WINPR_ATTR_NODISCARD +static UINT rail_send(RailServerContext* context, wStream* s, size_t length) { UINT status = CHANNEL_RC_OK; ULONG written = 0; - if (!context) - return CHANNEL_RC_BAD_INIT_HANDLE; + const ULONG ulen = WINPR_ASSERTING_INT_CAST(ULONG, length); + WINPR_ASSERT(context); - if (!WTSVirtualChannelWrite(context->priv->rail_channel, Stream_BufferAs(s, char), length, + if (!WTSVirtualChannelWrite(context->priv->rail_channel, Stream_BufferAs(s, char), ulen, &written)) { WLog_ERR(TAG, "WTSVirtualChannelWrite failed!"); @@ -61,21 +62,21 @@ static UINT rail_send(RailServerContext* context, wStream* s, ULONG length) * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_server_send_pdu(RailServerContext* context, wStream* s, UINT16 orderType) { char buffer[128] = WINPR_C_ARRAY_INIT; - UINT16 orderLength = 0; - if (!context || !s) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(context); + WINPR_ASSERT(s); - orderLength = (UINT16)Stream_GetPosition(s); + const size_t orderLength = Stream_GetPosition(s); Stream_ResetPosition(s); - if (!rail_write_pdu_header(s, orderType, orderLength)) + if (!rail_write_pdu_header(s, orderType, WINPR_ASSERTING_INT_CAST(UINT16, orderLength))) goto fail; if (!Stream_SetPosition(s, orderLength)) goto fail; - WLog_DBG(TAG, "Sending %s PDU, length: %" PRIu16 "", + WLog_DBG(TAG, "Sending %s PDU, length: %" PRIuz "", rail_get_order_type_string_full(orderType, buffer, sizeof(buffer)), orderLength); return rail_send(context, s, orderLength); @@ -84,34 +85,25 @@ fail: return ERROR_INVALID_DATA; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT rail_write_local_move_size_order(wStream* s, +static void rail_write_local_move_size_order(wStream* s, const RAIL_LOCALMOVESIZE_ORDER* localMoveSize) { - if (!s || !localMoveSize) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(localMoveSize); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 12)); Stream_Write_UINT32(s, localMoveSize->windowId); /* WindowId (4 bytes) */ Stream_Write_UINT16(s, localMoveSize->isMoveSizeStart ? 1 : 0); /* IsMoveSizeStart (2 bytes) */ Stream_Write_UINT16(s, localMoveSize->moveSizeType); /* MoveSizeType (2 bytes) */ Stream_Write_INT16(s, localMoveSize->posX); /* PosX (2 bytes) */ Stream_Write_INT16(s, localMoveSize->posY); /* PosY (2 bytes) */ - return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT rail_write_min_max_info_order(wStream* s, const RAIL_MINMAXINFO_ORDER* minMaxInfo) +static void rail_write_min_max_info_order(wStream* s, const RAIL_MINMAXINFO_ORDER* minMaxInfo) { - if (!s || !minMaxInfo) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(minMaxInfo); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 20)); Stream_Write_UINT32(s, minMaxInfo->windowId); /* WindowId (4 bytes) */ Stream_Write_INT16(s, minMaxInfo->maxWidth); /* MaxWidth (2 bytes) */ @@ -122,37 +114,26 @@ static UINT rail_write_min_max_info_order(wStream* s, const RAIL_MINMAXINFO_ORDE Stream_Write_INT16(s, minMaxInfo->minTrackHeight); /* MinTrackHeight (2 bytes) */ Stream_Write_INT16(s, minMaxInfo->maxTrackWidth); /* MaxTrackWidth (2 bytes) */ Stream_Write_INT16(s, minMaxInfo->maxTrackHeight); /* MaxTrackHeight (2 bytes) */ - return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT rail_write_taskbar_info_order(wStream* s, const RAIL_TASKBAR_INFO_ORDER* taskbarInfo) +static void rail_write_taskbar_info_order(wStream* s, const RAIL_TASKBAR_INFO_ORDER* taskbarInfo) { - if (!s || !taskbarInfo) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(taskbarInfo); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 12)); Stream_Write_UINT32(s, taskbarInfo->TaskbarMessage); /* TaskbarMessage (4 bytes) */ Stream_Write_UINT32(s, taskbarInfo->WindowIdTab); /* WindowIdTab (4 bytes) */ Stream_Write_UINT32(s, taskbarInfo->Body); /* Body (4 bytes) */ - return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT rail_write_langbar_info_order(wStream* s, const RAIL_LANGBAR_INFO_ORDER* langbarInfo) +static void rail_write_langbar_info_order(wStream* s, const RAIL_LANGBAR_INFO_ORDER* langbarInfo) { - if (!s || !langbarInfo) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(langbarInfo); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 4)); Stream_Write_UINT32(s, langbarInfo->languageBarStatus); /* LanguageBarStatus (4 bytes) */ - return ERROR_SUCCESS; } /** @@ -160,10 +141,11 @@ static UINT rail_write_langbar_info_order(wStream* s, const RAIL_LANGBAR_INFO_OR * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_write_exec_result_order(wStream* s, const RAIL_EXEC_RESULT_ORDER* execResult) { - if (!s || !execResult) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(execResult); if (execResult->exeOrFile.length > 520 || execResult->exeOrFile.length < 1) return ERROR_INVALID_DATA; @@ -178,61 +160,43 @@ static UINT rail_write_exec_result_order(wStream* s, const RAIL_EXEC_RESULT_ORDE return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT rail_write_z_order_sync_order(wStream* s, const RAIL_ZORDER_SYNC* zOrderSync) +static void rail_write_z_order_sync_order(wStream* s, const RAIL_ZORDER_SYNC* zOrderSync) { - if (!s || !zOrderSync) - return ERROR_INVALID_PARAMETER; - + WINPR_ASSERT(s); + WINPR_ASSERT(zOrderSync); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 4)); Stream_Write_UINT32(s, zOrderSync->windowIdMarker); /* WindowIdMarker (4 bytes) */ - return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT rail_write_cloak_order(wStream* s, const RAIL_CLOAK* cloak) +static void rail_write_cloak_order(wStream* s, const RAIL_CLOAK* cloak) { - if (!s || !cloak) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(cloak); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 5)); Stream_Write_UINT32(s, cloak->windowId); /* WindowId (4 bytes) */ Stream_Write_UINT8(s, cloak->cloak ? 1 : 0); /* Cloaked (1 byte) */ - return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ -static UINT +static void rail_write_power_display_request_order(wStream* s, const RAIL_POWER_DISPLAY_REQUEST* powerDisplayRequest) { - if (!s || !powerDisplayRequest) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(powerDisplayRequest); + WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 4)); Stream_Write_UINT32(s, powerDisplayRequest->active ? 1 : 0); /* Active (4 bytes) */ - return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ +WINPR_ATTR_NODISCARD static UINT rail_write_get_app_id_resp_order(wStream* s, const RAIL_GET_APPID_RESP_ORDER* getAppidResp) { - if (!s || !getAppidResp) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(getAppidResp); + if (!Stream_EnsureRemainingCapacity(s, 4ull + ARRAYSIZE(getAppidResp->applicationId))) + return ERROR_OUTOFMEMORY; Stream_Write_UINT32(s, getAppidResp->windowId); /* WindowId (4 bytes) */ if (!Stream_Write_UTF16_String( @@ -242,16 +206,16 @@ static UINT rail_write_get_app_id_resp_order(wStream* s, return ERROR_SUCCESS; } -/** - * Function description - * - * @return 0 on success, otherwise a Win32 error code - */ +WINPR_ATTR_NODISCARD static UINT rail_write_get_appid_resp_ex_order(wStream* s, const RAIL_GET_APPID_RESP_EX* getAppidRespEx) { - if (!s || !getAppidRespEx) - return ERROR_INVALID_PARAMETER; + WINPR_ASSERT(s); + WINPR_ASSERT(getAppidRespEx); + + if (!Stream_EnsureRemainingCapacity(s, (8ull + ARRAYSIZE(getAppidRespEx->applicationID) + + ARRAYSIZE(getAppidRespEx->processImageName)))) + return ERROR_OUTOFMEMORY; Stream_Write_UINT32(s, getAppidRespEx->windowID); /* WindowId (4 bytes) */ if (!Stream_Write_UTF16_String( @@ -271,16 +235,14 @@ static UINT rail_write_get_appid_resp_ex_order(wStream* s, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_handshake(RailServerContext* context, const RAIL_HANDSHAKE_ORDER* handshake) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !handshake) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_HANDSHAKE_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_HANDSHAKE_ORDER_LENGTH); if (!s) { @@ -289,9 +251,7 @@ static UINT rail_send_server_handshake(RailServerContext* context, } rail_write_handshake_order(s, handshake); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_HANDSHAKE); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_HANDSHAKE); } /** @@ -299,16 +259,14 @@ static UINT rail_send_server_handshake(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_handshake_ex(RailServerContext* context, const RAIL_HANDSHAKE_EX_ORDER* handshakeEx) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !handshakeEx || !context->priv) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_HANDSHAKE_EX_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_HANDSHAKE_EX_ORDER_LENGTH); if (!s) { @@ -317,11 +275,8 @@ static UINT rail_send_server_handshake_ex(RailServerContext* context, } rail_server_set_handshake_ex_flags(context, handshakeEx->railHandshakeFlags); - rail_write_handshake_ex_order(s, handshakeEx); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_HANDSHAKE_EX); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_HANDSHAKE_EX); } /** @@ -329,24 +284,20 @@ static UINT rail_send_server_handshake_ex(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_sysparam(RailServerContext* context, const RAIL_SYSPARAM_ORDER* sysparam) { - wStream* s = nullptr; - UINT error = 0; - RailServerPrivate* priv = nullptr; - BOOL extendedSpiSupported = 0; - if (!context || !sysparam) return ERROR_INVALID_PARAMETER; - priv = context->priv; + RailServerPrivate* priv = context->priv; if (!priv) return ERROR_INVALID_PARAMETER; - extendedSpiSupported = rail_is_extended_spi_supported(context->priv->channelFlags); - s = rail_pdu_init(RAIL_SYSPARAM_ORDER_LENGTH); + const BOOL extendedSpiSupported = rail_is_extended_spi_supported(context->priv->channelFlags); + wStream* s = rail_pdu_init(RAIL_SYSPARAM_ORDER_LENGTH); if (!s) { @@ -354,11 +305,13 @@ static UINT rail_send_server_sysparam(RailServerContext* context, return CHANNEL_RC_NO_MEMORY; } - error = rail_write_sysparam_order(s, sysparam, extendedSpiSupported); - if (error == CHANNEL_RC_OK) - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_SYSPARAM); - Stream_Free(s, TRUE); - return error; + const UINT error = rail_write_sysparam_order(s, sysparam, extendedSpiSupported); + if (error != CHANNEL_RC_OK) + { + Stream_Free(s, TRUE); + return error; + } + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_SYSPARAM); } /** @@ -366,16 +319,14 @@ static UINT rail_send_server_sysparam(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_local_move_size(RailServerContext* context, const RAIL_LOCALMOVESIZE_ORDER* localMoveSize) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !localMoveSize) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_LOCALMOVESIZE_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_LOCALMOVESIZE_ORDER_LENGTH); if (!s) { @@ -384,9 +335,7 @@ static UINT rail_send_server_local_move_size(RailServerContext* context, } rail_write_local_move_size_order(s, localMoveSize); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_LOCALMOVESIZE); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_LOCALMOVESIZE); } /** @@ -394,16 +343,14 @@ static UINT rail_send_server_local_move_size(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_min_max_info(RailServerContext* context, const RAIL_MINMAXINFO_ORDER* minMaxInfo) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !minMaxInfo) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_MINMAXINFO_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_MINMAXINFO_ORDER_LENGTH); if (!s) { @@ -412,9 +359,7 @@ static UINT rail_send_server_min_max_info(RailServerContext* context, } rail_write_min_max_info_order(s, minMaxInfo); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_MINMAXINFO); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_MINMAXINFO); } /** @@ -422,16 +367,14 @@ static UINT rail_send_server_min_max_info(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_taskbar_info(RailServerContext* context, const RAIL_TASKBAR_INFO_ORDER* taskbarInfo) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !taskbarInfo) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_TASKBAR_INFO_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_TASKBAR_INFO_ORDER_LENGTH); if (!s) { @@ -440,9 +383,7 @@ static UINT rail_send_server_taskbar_info(RailServerContext* context, } rail_write_taskbar_info_order(s, taskbarInfo); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_TASKBARINFO); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_TASKBARINFO); } /** @@ -450,16 +391,14 @@ static UINT rail_send_server_taskbar_info(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_langbar_info(RailServerContext* context, const RAIL_LANGBAR_INFO_ORDER* langbarInfo) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !langbarInfo) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_LANGBAR_INFO_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_LANGBAR_INFO_ORDER_LENGTH); if (!s) { @@ -468,9 +407,7 @@ static UINT rail_send_server_langbar_info(RailServerContext* context, } rail_write_langbar_info_order(s, langbarInfo); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_LANGBARINFO); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_LANGBARINFO); } /** @@ -478,16 +415,14 @@ static UINT rail_send_server_langbar_info(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_exec_result(RailServerContext* context, const RAIL_EXEC_RESULT_ORDER* execResult) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !execResult) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_EXEC_RESULT_ORDER_LENGTH + execResult->exeOrFile.length); + wStream* s = rail_pdu_init(RAIL_EXEC_RESULT_ORDER_LENGTH + execResult->exeOrFile.length); if (!s) { @@ -495,10 +430,13 @@ static UINT rail_send_server_exec_result(RailServerContext* context, return CHANNEL_RC_NO_MEMORY; } - rail_write_exec_result_order(s, execResult); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_EXEC_RESULT); - Stream_Free(s, TRUE); - return error; + const UINT error = rail_write_exec_result_order(s, execResult); + if (error != CHANNEL_RC_OK) + { + Stream_Free(s, TRUE); + return error; + } + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_EXEC_RESULT); } /** @@ -506,16 +444,14 @@ static UINT rail_send_server_exec_result(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_z_order_sync(RailServerContext* context, const RAIL_ZORDER_SYNC* zOrderSync) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !zOrderSync) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_Z_ORDER_SYNC_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_Z_ORDER_SYNC_ORDER_LENGTH); if (!s) { @@ -524,9 +460,7 @@ static UINT rail_send_server_z_order_sync(RailServerContext* context, } rail_write_z_order_sync_order(s, zOrderSync); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_ZORDER_SYNC); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_ZORDER_SYNC); } /** @@ -534,15 +468,13 @@ static UINT rail_send_server_z_order_sync(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_cloak(RailServerContext* context, const RAIL_CLOAK* cloak) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !cloak) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_CLOAK_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_CLOAK_ORDER_LENGTH); if (!s) { @@ -551,9 +483,7 @@ static UINT rail_send_server_cloak(RailServerContext* context, const RAIL_CLOAK* } rail_write_cloak_order(s, cloak); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_CLOAK); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_CLOAK); } /** @@ -561,17 +491,15 @@ static UINT rail_send_server_cloak(RailServerContext* context, const RAIL_CLOAK* * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_power_display_request(RailServerContext* context, const RAIL_POWER_DISPLAY_REQUEST* powerDisplayRequest) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !powerDisplayRequest) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_POWER_DISPLAY_REQUEST_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_POWER_DISPLAY_REQUEST_ORDER_LENGTH); if (!s) { @@ -580,9 +508,7 @@ rail_send_server_power_display_request(RailServerContext* context, } rail_write_power_display_request_order(s, powerDisplayRequest); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_POWER_DISPLAY_REQUEST); - Stream_Free(s, TRUE); - return error; + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_POWER_DISPLAY_REQUEST); } /** @@ -590,16 +516,14 @@ rail_send_server_power_display_request(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error coie */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_get_app_id_resp(RailServerContext* context, const RAIL_GET_APPID_RESP_ORDER* getAppidResp) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !getAppidResp) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_GET_APPID_RESP_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_GET_APPID_RESP_ORDER_LENGTH); if (!s) { @@ -607,10 +531,13 @@ static UINT rail_send_server_get_app_id_resp(RailServerContext* context, return CHANNEL_RC_NO_MEMORY; } - rail_write_get_app_id_resp_order(s, getAppidResp); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_GET_APPID_RESP); - Stream_Free(s, TRUE); - return error; + const UINT error = rail_write_get_app_id_resp_order(s, getAppidResp); + if (error != CHANNEL_RC_OK) + { + Stream_Free(s, TRUE); + return error; + } + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_GET_APPID_RESP); } /** @@ -618,16 +545,14 @@ static UINT rail_send_server_get_app_id_resp(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_send_server_get_appid_resp_ex(RailServerContext* context, const RAIL_GET_APPID_RESP_EX* getAppidRespEx) { - wStream* s = nullptr; - UINT error = 0; - if (!context || !getAppidRespEx) return ERROR_INVALID_PARAMETER; - s = rail_pdu_init(RAIL_GET_APPID_RESP_EX_ORDER_LENGTH); + wStream* s = rail_pdu_init(RAIL_GET_APPID_RESP_EX_ORDER_LENGTH); if (!s) { @@ -635,10 +560,13 @@ static UINT rail_send_server_get_appid_resp_ex(RailServerContext* context, return CHANNEL_RC_NO_MEMORY; } - rail_write_get_appid_resp_ex_order(s, getAppidRespEx); - error = rail_server_send_pdu(context, s, TS_RAIL_ORDER_GET_APPID_RESP_EX); - Stream_Free(s, TRUE); - return error; + const UINT error = rail_write_get_appid_resp_ex_order(s, getAppidRespEx); + if (error != CHANNEL_RC_OK) + { + Stream_Free(s, TRUE); + return error; + } + return rail_server_send_pdu(context, s, TS_RAIL_ORDER_GET_APPID_RESP_EX); } /** @@ -646,6 +574,7 @@ static UINT rail_send_server_get_appid_resp_ex(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_client_status_order(wStream* s, RAIL_CLIENT_STATUS_ORDER* clientStatus) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_CLIENT_STATUS_ORDER_LENGTH)) @@ -660,20 +589,18 @@ static UINT rail_read_client_status_order(wStream* s, RAIL_CLIENT_STATUS_ORDER* * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_exec_order(wStream* s, RAIL_EXEC_ORDER* exec, char* args[]) { RAIL_EXEC_ORDER order = WINPR_C_ARRAY_INIT; - UINT16 exeLen = 0; - UINT16 workLen = 0; - UINT16 argLen = 0; if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_EXEC_ORDER_LENGTH)) return ERROR_INVALID_DATA; - Stream_Read_UINT16(s, exec->flags); /* Flags (2 bytes) */ - Stream_Read_UINT16(s, exeLen); /* ExeOrFileLength (2 bytes) */ - Stream_Read_UINT16(s, workLen); /* WorkingDirLength (2 bytes) */ - Stream_Read_UINT16(s, argLen); /* ArgumentsLength (2 bytes) */ + exec->flags = Stream_Get_UINT16(s); /* Flags (2 bytes) */ + const UINT16 exeLen = Stream_Get_UINT16(s); /* ExeOrFileLength (2 bytes) */ + const UINT16 workLen = Stream_Get_UINT16(s); /* WorkingDirLength (2 bytes) */ + const UINT16 argLen = Stream_Get_UINT16(s); /* ArgumentsLength (2 bytes) */ if (!Stream_CheckAndLogRequiredLength(TAG, s, (size_t)exeLen + workLen + argLen)) return ERROR_INVALID_DATA; @@ -717,6 +644,7 @@ fail: * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_activate_order(wStream* s, RAIL_ACTIVATE_ORDER* activate) { BYTE enabled = 0; @@ -735,6 +663,7 @@ static UINT rail_read_activate_order(wStream* s, RAIL_ACTIVATE_ORDER* activate) * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_sysmenu_order(wStream* s, RAIL_SYSMENU_ORDER* sysmenu) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_SYSMENU_ORDER_LENGTH)) @@ -751,6 +680,7 @@ static UINT rail_read_sysmenu_order(wStream* s, RAIL_SYSMENU_ORDER* sysmenu) * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_syscommand_order(wStream* s, RAIL_SYSCOMMAND_ORDER* syscommand) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_SYSCOMMAND_ORDER_LENGTH)) @@ -766,6 +696,7 @@ static UINT rail_read_syscommand_order(wStream* s, RAIL_SYSCOMMAND_ORDER* syscom * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_notify_event_order(wStream* s, RAIL_NOTIFY_EVENT_ORDER* notifyEvent) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_NOTIFY_EVENT_ORDER_LENGTH)) @@ -782,6 +713,7 @@ static UINT rail_read_notify_event_order(wStream* s, RAIL_NOTIFY_EVENT_ORDER* no * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_get_appid_req_order(wStream* s, RAIL_GET_APPID_REQ_ORDER* getAppidReq) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_GET_APPID_REQ_ORDER_LENGTH)) @@ -796,6 +728,7 @@ static UINT rail_read_get_appid_req_order(wStream* s, RAIL_GET_APPID_REQ_ORDER* * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_window_move_order(wStream* s, RAIL_WINDOW_MOVE_ORDER* windowMove) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_WINDOW_MOVE_ORDER_LENGTH)) @@ -814,6 +747,7 @@ static UINT rail_read_window_move_order(wStream* s, RAIL_WINDOW_MOVE_ORDER* wind * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_snap_arange_order(wStream* s, RAIL_SNAP_ARRANGE* snapArrange) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_SNAP_ARRANGE_ORDER_LENGTH)) @@ -832,6 +766,7 @@ static UINT rail_read_snap_arange_order(wStream* s, RAIL_SNAP_ARRANGE* snapArran * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_langbar_info_order(wStream* s, RAIL_LANGBAR_INFO_ORDER* langbarInfo) { if (!Stream_CheckAndLogRequiredLength(TAG, s, RAIL_LANGBAR_INFO_ORDER_LENGTH)) @@ -846,6 +781,7 @@ static UINT rail_read_langbar_info_order(wStream* s, RAIL_LANGBAR_INFO_ORDER* la * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_language_ime_info_order(wStream* s, RAIL_LANGUAGEIME_INFO_ORDER* languageImeInfo) { @@ -868,6 +804,7 @@ static UINT rail_read_language_ime_info_order(wStream* s, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_compartment_info_order(wStream* s, RAIL_COMPARTMENT_INFO_ORDER* compartmentInfo) { @@ -886,6 +823,7 @@ static UINT rail_read_compartment_info_order(wStream* s, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_read_cloak_order(wStream* s, RAIL_CLOAK* cloak) { BYTE cloaked = 0; @@ -904,10 +842,11 @@ static UINT rail_read_cloak_order(wStream* s, RAIL_CLOAK* cloak) * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_handshake_order(RailServerContext* context, RAIL_HANDSHAKE_ORDER* handshake, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !handshake || !s) return ERROR_INVALID_PARAMETER; @@ -931,10 +870,11 @@ static UINT rail_recv_client_handshake_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_client_status_order(RailServerContext* context, RAIL_CLIENT_STATUS_ORDER* clientStatus, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !clientStatus || !s) return ERROR_INVALID_PARAMETER; @@ -958,16 +898,16 @@ static UINT rail_recv_client_client_status_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_exec_order(RailServerContext* context, wStream* s) { - UINT error = 0; char* args[3] = WINPR_C_ARRAY_INIT; RAIL_EXEC_ORDER exec = WINPR_C_ARRAY_INIT; if (!context || !s) return ERROR_INVALID_PARAMETER; - error = rail_read_exec_order(s, &exec, args); + UINT error = rail_read_exec_order(s, &exec, args); if (error) { WLog_ERR(TAG, "rail_read_client_status_order failed with error %" PRIu32 "!", error); @@ -990,17 +930,16 @@ static UINT rail_recv_client_exec_order(RailServerContext* context, wStream* s) * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_sysparam_order(RailServerContext* context, RAIL_SYSPARAM_ORDER* sysparam, wStream* s) { - UINT error = 0; - BOOL extendedSpiSupported = 0; - if (!context || !sysparam || !s) return ERROR_INVALID_PARAMETER; - extendedSpiSupported = rail_is_extended_spi_supported(context->priv->channelFlags); - if ((error = rail_read_sysparam_order(s, sysparam, extendedSpiSupported))) + const BOOL extendedSpiSupported = rail_is_extended_spi_supported(context->priv->channelFlags); + UINT error = rail_read_sysparam_order(s, sysparam, extendedSpiSupported); + if (error != CHANNEL_RC_OK) { WLog_ERR(TAG, "rail_read_sysparam_order failed with error %" PRIu32 "!", error); return error; @@ -1019,10 +958,11 @@ static UINT rail_recv_client_sysparam_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_activate_order(RailServerContext* context, RAIL_ACTIVATE_ORDER* activate, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !activate || !s) return ERROR_INVALID_PARAMETER; @@ -1046,10 +986,11 @@ static UINT rail_recv_client_activate_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_sysmenu_order(RailServerContext* context, RAIL_SYSMENU_ORDER* sysmenu, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !sysmenu || !s) return ERROR_INVALID_PARAMETER; @@ -1073,10 +1014,11 @@ static UINT rail_recv_client_sysmenu_order(RailServerContext* context, RAIL_SYSM * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_syscommand_order(RailServerContext* context, RAIL_SYSCOMMAND_ORDER* syscommand, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !syscommand || !s) return ERROR_INVALID_PARAMETER; @@ -1100,10 +1042,11 @@ static UINT rail_recv_client_syscommand_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_notify_event_order(RailServerContext* context, RAIL_NOTIFY_EVENT_ORDER* notifyEvent, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !notifyEvent || !s) return ERROR_INVALID_PARAMETER; @@ -1127,10 +1070,11 @@ static UINT rail_recv_client_notify_event_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_window_move_order(RailServerContext* context, RAIL_WINDOW_MOVE_ORDER* windowMove, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !windowMove || !s) return ERROR_INVALID_PARAMETER; @@ -1154,10 +1098,11 @@ static UINT rail_recv_client_window_move_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_snap_arrange_order(RailServerContext* context, RAIL_SNAP_ARRANGE* snapArrange, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !snapArrange || !s) return ERROR_INVALID_PARAMETER; @@ -1181,10 +1126,11 @@ static UINT rail_recv_client_snap_arrange_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_get_appid_req_order(RailServerContext* context, RAIL_GET_APPID_REQ_ORDER* getAppidReq, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !getAppidReq || !s) return ERROR_INVALID_PARAMETER; @@ -1208,10 +1154,11 @@ static UINT rail_recv_client_get_appid_req_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_langbar_info_order(RailServerContext* context, RAIL_LANGBAR_INFO_ORDER* langbarInfo, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !langbarInfo || !s) return ERROR_INVALID_PARAMETER; @@ -1235,11 +1182,12 @@ static UINT rail_recv_client_langbar_info_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_language_ime_info_order(RailServerContext* context, RAIL_LANGUAGEIME_INFO_ORDER* languageImeInfo, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !languageImeInfo || !s) return ERROR_INVALID_PARAMETER; @@ -1263,11 +1211,12 @@ static UINT rail_recv_client_language_ime_info_order(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_compartment_info(RailServerContext* context, RAIL_COMPARTMENT_INFO_ORDER* compartmentInfo, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !compartmentInfo || !s) return ERROR_INVALID_PARAMETER; @@ -1291,9 +1240,10 @@ static UINT rail_recv_client_compartment_info(RailServerContext* context, * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_recv_client_cloak_order(RailServerContext* context, RAIL_CLOAK* cloak, wStream* s) { - UINT error = 0; + UINT error = CHANNEL_RC_OK; if (!context || !cloak || !s) return ERROR_INVALID_PARAMETER; @@ -1312,6 +1262,7 @@ static UINT rail_recv_client_cloak_order(RailServerContext* context, RAIL_CLOAK* return error; } +WINPR_ATTR_NODISCARD static UINT rail_recv_client_text_scale_order(RailServerContext* context, wStream* s) { UINT error = CHANNEL_RC_OK; @@ -1332,6 +1283,7 @@ static UINT rail_recv_client_text_scale_order(RailServerContext* context, wStrea return error; } +WINPR_ATTR_NODISCARD static UINT rail_recv_client_caret_blink(RailServerContext* context, wStream* s) { UINT error = CHANNEL_RC_OK; @@ -1352,10 +1304,15 @@ static UINT rail_recv_client_caret_blink(RailServerContext* context, wStream* s) return error; } +WINPR_ATTR_NODISCARD static DWORD WINAPI rail_server_thread(LPVOID arg) { RailServerContext* context = (RailServerContext*)arg; + WINPR_ASSERT(context); + RailServerPrivate* priv = context->priv; + WINPR_ASSERT(priv); + DWORD status = 0; DWORD nCount = 0; HANDLE events[8]; @@ -1421,12 +1378,18 @@ static DWORD WINAPI rail_server_thread(LPVOID arg) * * @return 0 on success, otherwise a Win32 error code */ +WINPR_ATTR_NODISCARD static UINT rail_server_start(RailServerContext* context) { void* buffer = nullptr; DWORD bytesReturned = 0; - RailServerPrivate* priv = context->priv; UINT error = ERROR_INTERNAL_ERROR; + + WINPR_ASSERT(context); + + RailServerPrivate* priv = context->priv; + WINPR_ASSERT(priv); + priv->rail_channel = WTSVirtualChannelOpen(context->vcm, WTS_CURRENT_SESSION, RAIL_SVC_CHANNEL_NAME); @@ -1480,8 +1443,10 @@ out_close: return error; } +WINPR_ATTR_NODISCARD static BOOL rail_server_stop(RailServerContext* context) { + WINPR_ASSERT(context); RailServerPrivate* priv = context->priv; if (priv->thread) @@ -1543,7 +1508,7 @@ RailServerContext* rail_server_context_new(HANDLE vcm) if (!priv) { WLog_ERR(TAG, "calloc failed!"); - goto out_free; + goto fail; } /* Create shared input stream */ @@ -1552,14 +1517,12 @@ RailServerContext* rail_server_context_new(HANDLE vcm) if (!priv->input_stream) { WLog_ERR(TAG, "Stream_New failed!"); - goto out_free_priv; + goto fail; } return context; -out_free_priv: - free(context->priv); -out_free: - free(context); +fail: + rail_server_context_free(context); return nullptr; } @@ -1574,12 +1537,10 @@ void rail_server_context_free(RailServerContext* context) void rail_server_set_handshake_ex_flags(RailServerContext* context, DWORD flags) { - RailServerPrivate* priv = nullptr; + WINPR_ASSERT(context); + WINPR_ASSERT(context->priv); - if (!context || !context->priv) - return; - - priv = context->priv; + RailServerPrivate* priv = context->priv; priv->channelFlags = flags; } diff --git a/include/freerdp/server/rail.h b/include/freerdp/server/rail.h index 07a2bb7e2..9bb533695 100644 --- a/include/freerdp/server/rail.h +++ b/include/freerdp/server/rail.h @@ -145,7 +145,6 @@ extern "C" FREERDP_API void rail_server_context_free(RailServerContext* context); WINPR_ATTR_MALLOC(rail_server_context_free, 1) - WINPR_ATTR_NODISCARD FREERDP_API RailServerContext* rail_server_context_new(HANDLE vcm); WINPR_ATTR_NODISCARD From d34f41a30b0e503d0494bc11cd0146172157e809 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 08:50:26 +0100 Subject: [PATCH 2/6] [winpr,sspi] fix missing return check --- winpr/libwinpr/sspi/NTLM/ntlm.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/winpr/libwinpr/sspi/NTLM/ntlm.c b/winpr/libwinpr/sspi/NTLM/ntlm.c index 759712c92..9f73343f0 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm.c @@ -391,8 +391,12 @@ static SECURITY_STATUS SEC_ENTRY ntlm_AcquireCredentialsHandleW( { UINT32 identityFlags = sspi_GetAuthIdentityFlags(pAuthData); - sspi_CopyAuthIdentity(&(credentials->identity), - (const SEC_WINNT_AUTH_IDENTITY_INFO*)pAuthData); + if (sspi_CopyAuthIdentity(&(credentials->identity), + (const SEC_WINNT_AUTH_IDENTITY_INFO*)pAuthData) < 0) + { + sspi_CredentialsFree(credentials); + return SEC_E_INVALID_PARAMETER; + } if (identityFlags & SEC_WINNT_AUTH_IDENTITY_EXTENDED) settings = (((SEC_WINNT_AUTH_IDENTITY_WINPR*)pAuthData)->ntlmSettings); @@ -1233,7 +1237,14 @@ static SECURITY_STATUS SEC_ENTRY ntlm_DecryptMessage(PCtxtHandle phContext, PSec /* Decrypt message using with RC4, result overwrites original buffer */ if (context->confidentiality) - winpr_RC4_Update(context->RecvRc4Seal, length, (BYTE*)data, (BYTE*)data_buffer->pvBuffer); + { + if (!winpr_RC4_Update(context->RecvRc4Seal, length, (BYTE*)data, + (BYTE*)data_buffer->pvBuffer)) + { + free(data); + return SEC_E_INSUFFICIENT_MEMORY; + } + } else CopyMemory(data_buffer->pvBuffer, data, length); From 23fc8778cb76e70af922997d9358cc64bbf538fc Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 08:49:18 +0100 Subject: [PATCH 3/6] [channels,usb] fix possible deadlock newly introduced error handling did lead to a deadlock. refactor to avoid this. --- channels/urbdrc/client/libusb/libusb_udevice.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/channels/urbdrc/client/libusb/libusb_udevice.c b/channels/urbdrc/client/libusb/libusb_udevice.c index cb7178556..af09e281f 100644 --- a/channels/urbdrc/client/libusb/libusb_udevice.c +++ b/channels/urbdrc/client/libusb/libusb_udevice.c @@ -267,7 +267,7 @@ static void LIBUSB_CALL func_iso_callback(struct libusb_transfer* transfer) BYTE* dataStart = Stream_Pointer(user_data->data); if (!Stream_SetPosition(user_data->data, 40)) /* TS_URB_ISOCH_TRANSFER_RESULT IsoPacket offset */ - return; + break; for (uint32_t i = 0; i < WINPR_ASSERTING_INT_CAST(uint32_t, transfer->num_iso_packets); i++) From 4475e21b7eab24f71ad214e7a3cb6993d2ef4b80 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 08:32:15 +0100 Subject: [PATCH 4/6] [codec,rfx] return nullptr if input parameters are invalid rfx_encode_messages maxDataSize must be > 1024 or the function will fail. Add a check to abort early if this is the case. while this is a usage error, it is helpful to have proper error handling in the function as well. --- libfreerdp/codec/rfx.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c index b0c322c5d..cbdf44668 100644 --- a/libfreerdp/codec/rfx.c +++ b/libfreerdp/codec/rfx.c @@ -1952,6 +1952,9 @@ static inline RFX_MESSAGE* rfx_split_message(RFX_CONTEXT* WINPR_RESTRICT context WINPR_ASSERT(message); WINPR_ASSERT(numMessages); + if (maxDataSize <= 1024) + return nullptr; + maxDataSize -= 1024; /* reserve enough space for headers */ *numMessages = ((message->tilesDataSize + maxDataSize) / maxDataSize) * 4ull; From 103e0907ccca91d28c5bb02dc231da4d5da46f99 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 2 Mar 2026 20:16:43 +0100 Subject: [PATCH 5/6] [core,caps] use getter/setter for MultifragMaxRequestSize to ease debugging use the getter/setters instead of direct struct access. --- libfreerdp/core/capabilities.c | 57 ++++++++++++++++++++++------------ libfreerdp/core/fastpath.c | 14 ++++++--- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/libfreerdp/core/capabilities.c b/libfreerdp/core/capabilities.c index 5f97c14fd..850078d07 100644 --- a/libfreerdp/core/capabilities.c +++ b/libfreerdp/core/capabilities.c @@ -2706,12 +2706,11 @@ static BOOL rdp_print_desktop_composition_capability_set(wLog* log, wStream* s) static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings, const rdpSettings* src) { - UINT32 multifragMaxRequestSize = 0; - WINPR_ASSERT(settings); WINPR_ASSERT(src); - multifragMaxRequestSize = src->MultifragMaxRequestSize; + UINT32 multifragMaxRequestSize = + freerdp_settings_get_uint32(src, FreeRDP_MultifragMaxRequestSize); if (settings->ServerMode) { @@ -2733,14 +2732,17 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings, * than or equal to the value we've previously sent in the server to * client multi-fragment update capability set (MS-RDPRFX 1.5) */ - if (multifragMaxRequestSize < settings->MultifragMaxRequestSize) + if (multifragMaxRequestSize < + freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize)) { /* * If it happens to be smaller we honor the client's value but * have to disable RemoteFX */ settings->RemoteFxCodec = FALSE; - settings->MultifragMaxRequestSize = multifragMaxRequestSize; + if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize, + multifragMaxRequestSize)) + return FALSE; } else { @@ -2749,7 +2751,9 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings, } else { - settings->MultifragMaxRequestSize = multifragMaxRequestSize; + if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize, + multifragMaxRequestSize)) + return FALSE; } } else @@ -2759,8 +2763,13 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings, * In RemoteFX mode we MUST do this but it might also be useful to * receive larger related bitmap updates. */ - if (multifragMaxRequestSize > settings->MultifragMaxRequestSize) - settings->MultifragMaxRequestSize = multifragMaxRequestSize; + if (multifragMaxRequestSize > + freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize)) + { + if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize, + multifragMaxRequestSize)) + return FALSE; + } } return TRUE; } @@ -2773,16 +2782,13 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings, static BOOL rdp_read_multifragment_update_capability_set(wLog* log, wStream* s, rdpSettings* settings) { - UINT32 multifragMaxRequestSize = 0; - WINPR_ASSERT(settings); if (!Stream_CheckAndLogRequiredLengthWLog(log, s, 4)) return FALSE; - Stream_Read_UINT32(s, multifragMaxRequestSize); /* MaxRequestSize (4 bytes) */ - settings->MultifragMaxRequestSize = multifragMaxRequestSize; - - return TRUE; + const UINT32 multifragMaxRequestSize = Stream_Get_UINT32(s); /* MaxRequestSize (4 bytes) */ + return freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize, + multifragMaxRequestSize); } /* @@ -2794,7 +2800,8 @@ static BOOL rdp_write_multifragment_update_capability_set(wLog* log, wStream* s, rdpSettings* settings) { WINPR_ASSERT(settings); - if (settings->ServerMode && settings->MultifragMaxRequestSize == 0) + if (settings->ServerMode && + (freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize) == 0)) { /* * In server mode we prefer to use the highest useful request size that @@ -2807,11 +2814,19 @@ static BOOL rdp_write_multifragment_update_capability_set(wLog* log, wStream* s, * greater than or equal to the value we're sending here. * See [MS-RDPRFX 1.5 capability #2] */ - UINT32 tileNumX = (settings->DesktopWidth + 63) / 64; - UINT32 tileNumY = (settings->DesktopHeight + 63) / 64; - settings->MultifragMaxRequestSize = tileNumX * tileNumY * 16384; + const UINT32 tileNumX = (settings->DesktopWidth + 63) / 64; + const UINT32 tileNumY = (settings->DesktopHeight + 63) / 64; + + WINPR_ASSERT(tileNumX < UINT32_MAX / tileNumY); + WINPR_ASSERT(tileNumY < UINT32_MAX / tileNumX); + WINPR_ASSERT(tileNumX * tileNumY < UINT32_MAX / 16384u); + /* and add room for headers, regions, frame markers, etc. */ - settings->MultifragMaxRequestSize += 16384; + const UINT32 MultifragMaxRequestSize = (tileNumX * tileNumY + 1u) * 16384u; + + if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize, + MultifragMaxRequestSize)) + return FALSE; } if (!Stream_EnsureRemainingCapacity(s, 32)) @@ -4791,7 +4806,9 @@ BOOL rdp_recv_confirm_active(rdpRdp* rdp, wStream* s, UINT16 pduLength) if (!settings->ReceivedCapabilities[CAPSET_TYPE_MULTI_FRAGMENT_UPDATE]) { /* client does not support multi fragment updates - make sure packages are not fragmented */ - settings->MultifragMaxRequestSize = FASTPATH_FRAGMENT_SAFE_SIZE; + if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize, + FASTPATH_FRAGMENT_SAFE_SIZE)) + return FALSE; } if (!settings->ReceivedCapabilities[CAPSET_TYPE_LARGE_POINTER]) diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c index 38ae0329a..87e7f0572 100644 --- a/libfreerdp/core/fastpath.c +++ b/libfreerdp/core/fastpath.c @@ -574,10 +574,13 @@ static int fastpath_recv_update_data(rdpFastPath* fastpath, wStream* s) WINPR_ASSERT(context); WINPR_ASSERT(context->settings); - if (totalSize > context->settings->MultifragMaxRequestSize) + if (totalSize > + freerdp_settings_get_uint32(context->settings, FreeRDP_MultifragMaxRequestSize)) { - WLog_ERR(TAG, "Total size (%" PRIuz ") exceeds MultifragMaxRequestSize (%" PRIu32 ")", - totalSize, context->settings->MultifragMaxRequestSize); + WLog_ERR( + TAG, "Total size (%" PRIuz ") exceeds MultifragMaxRequestSize (%" PRIu32 ")", + totalSize, + freerdp_settings_get_uint32(context->settings, FreeRDP_MultifragMaxRequestSize)); goto out_fail; } @@ -1211,12 +1214,13 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s } /* check if the client's fast path pdu buffer is large enough */ - if (totalLength > settings->MultifragMaxRequestSize) + if (totalLength > freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize)) { WLog_ERR(TAG, "fast path update size (%" PRIuz ") exceeds the client's maximum request size (%" PRIu32 ")", - totalLength, settings->MultifragMaxRequestSize); + totalLength, + freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize)); return FALSE; } From b724ba546d9c8ff477fa6a4f9f6583fd5d0e4ae1 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 2 Mar 2026 18:46:22 +0100 Subject: [PATCH 6/6] [checks,return] fix various unchecked return values --- channels/rdpgfx/client/rdpgfx_main.c | 3 ++- channels/urbdrc/client/libusb/libusb_udevman.c | 3 ++- channels/urbdrc/client/urbdrc_main.c | 3 ++- client/X11/cli/xfreerdp.c | 3 ++- client/X11/xf_window.c | 6 +++++- client/common/client_cliprdr_file.c | 13 +++++++++---- client/common/cmdline.c | 1 - libfreerdp/emu/scard/smartcard_virtual_gids.c | 8 +++++--- server/proxy/channels/pf_channel_smartcard.c | 5 ++++- server/proxy/pf_modules.c | 4 ++-- server/proxy/pf_server.c | 4 +++- server/proxy/proxy_modules.h | 2 +- winpr/include/winpr/collections.h | 1 - winpr/libwinpr/clipboard/synthetic.c | 5 ++++- winpr/libwinpr/clipboard/synthetic_file.c | 3 ++- winpr/libwinpr/utils/collections/MessagePipe.c | 16 +++++++--------- 16 files changed, 50 insertions(+), 30 deletions(-) diff --git a/channels/rdpgfx/client/rdpgfx_main.c b/channels/rdpgfx/client/rdpgfx_main.c index 925d9ab17..26b29d1b2 100644 --- a/channels/rdpgfx/client/rdpgfx_main.c +++ b/channels/rdpgfx/client/rdpgfx_main.c @@ -70,7 +70,8 @@ static BOOL delete_surface(const void* key, void* value, void* arg) static void free_surfaces(RdpgfxClientContext* context, wHashTable* SurfaceTable) { - HashTable_Foreach(SurfaceTable, delete_surface, context); + if (!HashTable_Foreach(SurfaceTable, delete_surface, context)) + WLog_WARN(TAG, "delete_surface failed"); } static UINT evict_cache_slots(RdpgfxClientContext* context, UINT16 MaxCacheSlots, void** CacheSlots) diff --git a/channels/urbdrc/client/libusb/libusb_udevman.c b/channels/urbdrc/client/libusb/libusb_udevman.c index 13aed81d9..4a640469b 100644 --- a/channels/urbdrc/client/libusb/libusb_udevman.c +++ b/channels/urbdrc/client/libusb/libusb_udevman.c @@ -723,7 +723,8 @@ static UINT urbdrc_udevman_parse_addin_args(UDEVMAN* udevman, const ADDIN_ARGV* const char* arg = args->argv[x]; if (strcmp(arg, "dbg") == 0) { - WLog_SetLogLevel(WLog_Get(TAG), WLOG_TRACE); + if (!WLog_SetLogLevel(WLog_Get(TAG), WLOG_TRACE)) + return ERROR_INTERNAL_ERROR; } else if (_strnicmp(arg, "device:", 7) == 0) { diff --git a/channels/urbdrc/client/urbdrc_main.c b/channels/urbdrc/client/urbdrc_main.c index 7cc437aa0..f50ca3933 100644 --- a/channels/urbdrc/client/urbdrc_main.c +++ b/channels/urbdrc/client/urbdrc_main.c @@ -890,7 +890,8 @@ static UINT urbdrc_process_addin_args(URBDRC_PLUGIN* urbdrc, const ADDIN_ARGV* a CommandLineSwitchStart(arg) CommandLineSwitchCase(arg, "dbg") { - WLog_SetLogLevel(urbdrc->log, WLOG_TRACE); + if (!WLog_SetLogLevel(urbdrc->log, WLOG_TRACE)) + return ERROR_INTERNAL_ERROR; } CommandLineSwitchCase(arg, "sys") { diff --git a/client/X11/cli/xfreerdp.c b/client/X11/cli/xfreerdp.c index cd173355c..e03757139 100644 --- a/client/X11/cli/xfreerdp.c +++ b/client/X11/cli/xfreerdp.c @@ -108,7 +108,8 @@ int main(int argc, char* argv[]) thread = freerdp_client_get_thread(context); (void)WaitForSingleObject(thread, INFINITE); - GetExitCodeThread(thread, &dwExitCode); + if (!GetExitCodeThread(thread, &dwExitCode)) + goto out; rc = xf_exit_code_from_disconnect_reason(dwExitCode); freerdp_client_stop(context); diff --git a/client/X11/xf_window.c b/client/X11/xf_window.c index 9cdec0b4d..03f725cb0 100644 --- a/client/X11/xf_window.c +++ b/client/X11/xf_window.c @@ -754,7 +754,11 @@ xfWindow* xf_CreateDesktopWindow(xfContext* xfc, char* name, int width, int heig LogDynAndXClearWindow(xfc->log, xfc->display, window->handle); xf_SetWindowTitleText(xfc, window->handle, name); LogDynAndXMapWindow(xfc->log, xfc->display, window->handle); - xf_input_init(xfc, window->handle); + if (!xf_input_init(xfc, window->handle)) + { + xf_DestroyDesktopWindow(xfc, window); + return nullptr; + } /* * NOTE: This must be done here to handle reparenting the window, diff --git a/client/common/client_cliprdr_file.c b/client/common/client_cliprdr_file.c index ce5f6491c..bbf0f67af 100644 --- a/client/common/client_cliprdr_file.c +++ b/client/common/client_cliprdr_file.c @@ -549,15 +549,20 @@ static BOOL clear_clip_data_entries(WINPR_ATTR_UNUSED const void* key, void* val return TRUE; } -static void clear_cdi_entries(CliprdrFileContext* file_context) +WINPR_ATTR_NODISCARD +static UINT clear_cdi_entries(CliprdrFileContext* file_context) { + UINT res = CHANNEL_RC_OK; WINPR_ASSERT(file_context); HashTable_Lock(file_context->inode_table); - HashTable_Foreach(file_context->clip_data_table, clear_clip_data_entries, nullptr); + if (!HashTable_Foreach(file_context->clip_data_table, clear_clip_data_entries, nullptr)) + res = ERROR_INTERNAL_ERROR; HashTable_Clear(file_context->clip_data_table); HashTable_Unlock(file_context->inode_table); + + return res; } static UINT prepare_clip_data_entry_with_id(CliprdrFileContext* file_context) @@ -616,7 +621,7 @@ UINT cliprdr_file_context_notify_new_server_format_list(CliprdrFileContext* file #if defined(WITH_FUSE) clear_no_cdi_entry(file_context); /* TODO: assign timeouts to old locks instead */ - clear_cdi_entries(file_context); + rc = clear_cdi_entries(file_context); if (does_server_support_clipdata_locking(file_context)) rc = prepare_clip_data_entry_with_id(file_context); @@ -634,7 +639,7 @@ UINT cliprdr_file_context_notify_new_client_format_list(CliprdrFileContext* file #if defined(WITH_FUSE) clear_no_cdi_entry(file_context); /* TODO: assign timeouts to old locks instead */ - clear_cdi_entries(file_context); + return clear_cdi_entries(file_context); #endif return CHANNEL_RC_OK; diff --git a/client/common/cmdline.c b/client/common/cmdline.c index 941cc9d22..8651d15fc 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -5606,7 +5606,6 @@ static int freerdp_client_settings_parse_command_line_arguments_int( if (!isArgsFrom) warn_credential_args(largs); - CommandLineFindArgumentA(largs, "v"); arg = largs; errno = 0; diff --git a/libfreerdp/emu/scard/smartcard_virtual_gids.c b/libfreerdp/emu/scard/smartcard_virtual_gids.c index 42e77cf81..3ec3053b2 100644 --- a/libfreerdp/emu/scard/smartcard_virtual_gids.c +++ b/libfreerdp/emu/scard/smartcard_virtual_gids.c @@ -663,9 +663,10 @@ static BOOL vgids_read_do_fkt(void* data, size_t index, va_list ap) return TRUE; } -static void vgids_read_do(vgidsContext* context, UINT16 efID, UINT16 doID) +WINPR_ATTR_NODISCARD +static BOOL vgids_read_do(vgidsContext* context, UINT16 efID, UINT16 doID) { - ArrayList_ForEach(context->files, vgids_read_do_fkt, context, efID, doID); + return ArrayList_ForEach(context->files, vgids_read_do_fkt, context, efID, doID); } static void vgids_reset_context_response(vgidsContext* context) @@ -897,7 +898,8 @@ static BOOL vgids_ins_getdata(vgidsContext* context, wStream* s, BYTE** response } Stream_Read_UINT16_BE(s, doId); - vgids_read_do(context, fileId, doId); + if (!vgids_read_do(context, fileId, doId)) + return FALSE; break; } case 0xA: diff --git a/server/proxy/channels/pf_channel_smartcard.c b/server/proxy/channels/pf_channel_smartcard.c index c2af7fd00..c6e2f8953 100644 --- a/server/proxy/channels/pf_channel_smartcard.c +++ b/server/proxy/channels/pf_channel_smartcard.c @@ -151,12 +151,15 @@ static BOOL start_irp_thread(pf_channel_client_context* scard, work = CreateThreadpoolWork(irp_thread, arg, nullptr); if (!work) goto fail; - ArrayList_Append(scard->workObjects, work); + if (!ArrayList_Append(scard->workObjects, work)) + goto fail; SubmitThreadpoolWork(work); return TRUE; fail: + if (work) + CloseThreadpoolWork(work); if (arg) queue_free(arg->e); free(arg); diff --git a/server/proxy/pf_modules.c b/server/proxy/pf_modules.c index dbfea2251..5e9ee6df8 100644 --- a/server/proxy/pf_modules.c +++ b/server/proxy/pf_modules.c @@ -483,7 +483,7 @@ static BOOL pf_modules_print_ArrayList_ForEachFkt(void* data, size_t index, va_l return TRUE; } -void pf_modules_list_loaded_plugins(proxyModule* module) +BOOL pf_modules_list_loaded_plugins(proxyModule* module) { size_t count = 0; @@ -495,7 +495,7 @@ void pf_modules_list_loaded_plugins(proxyModule* module) if (count > 0) WLog_INFO(TAG, "Loaded plugins:"); - ArrayList_ForEach(module->plugins, pf_modules_print_ArrayList_ForEachFkt); + return ArrayList_ForEach(module->plugins, pf_modules_print_ArrayList_ForEachFkt); } WINPR_ATTR_NODISCARD diff --git a/server/proxy/pf_server.c b/server/proxy/pf_server.c index 2df961929..3df04a6c1 100644 --- a/server/proxy/pf_server.c +++ b/server/proxy/pf_server.c @@ -970,7 +970,9 @@ proxyServer* pf_server_new(const proxyConfig* config) goto out; } - pf_modules_list_loaded_plugins(server->module); + if (!pf_modules_list_loaded_plugins(server->module)) + goto out; + if (!are_all_required_modules_loaded(server->module, server->config)) goto out; diff --git a/server/proxy/proxy_modules.h b/server/proxy/proxy_modules.h index 0979501af..1f30da515 100644 --- a/server/proxy/proxy_modules.h +++ b/server/proxy/proxy_modules.h @@ -90,7 +90,7 @@ extern "C" WINPR_ATTR_NODISCARD BOOL pf_modules_is_plugin_loaded(proxyModule* module, const char* plugin_name); - void pf_modules_list_loaded_plugins(proxyModule* module); + WINPR_ATTR_NODISCARD BOOL pf_modules_list_loaded_plugins(proxyModule* module); WINPR_ATTR_NODISCARD BOOL pf_modules_run_filter(proxyModule* module, PF_FILTER_TYPE type, proxyData* pdata, void* param); diff --git a/winpr/include/winpr/collections.h b/winpr/include/winpr/collections.h index b090f7a0f..a1795e132 100644 --- a/winpr/include/winpr/collections.h +++ b/winpr/include/winpr/collections.h @@ -822,7 +822,6 @@ extern "C" WINPR_API void MessagePipe_Free(wMessagePipe* pipe); WINPR_ATTR_MALLOC(MessagePipe_Free, 1) - WINPR_ATTR_NODISCARD WINPR_API wMessagePipe* MessagePipe_New(void); /* Publisher/Subscriber Pattern */ diff --git a/winpr/libwinpr/clipboard/synthetic.c b/winpr/libwinpr/clipboard/synthetic.c index 550a1a462..1fac5b693 100644 --- a/winpr/libwinpr/clipboard/synthetic.c +++ b/winpr/libwinpr/clipboard/synthetic.c @@ -653,7 +653,10 @@ static void* clipboard_synthesize_html_format(wClipboard* clipboard, UINT32 form /* Check the BOM (Byte Order Mark) */ if ((pSrcData.cpb[0] == 0xFE) && (pSrcData.cpb[1] == 0xFF)) - ByteSwapUnicode(pSrcData.pv, (SrcSize / 2)); + { + if (!ByteSwapUnicode(pSrcData.pv, (SrcSize / 2))) + goto fail; + } /* Check if we have WCHAR, convert to UTF-8 */ if ((pSrcData.cpb[0] == 0xFF) && (pSrcData.cpb[1] == 0xFE)) diff --git a/winpr/libwinpr/clipboard/synthetic_file.c b/winpr/libwinpr/clipboard/synthetic_file.c index a9214f749..502fa47d1 100644 --- a/winpr/libwinpr/clipboard/synthetic_file.c +++ b/winpr/libwinpr/clipboard/synthetic_file.c @@ -104,7 +104,8 @@ static struct synthetic_file* make_synthetic_file(const WCHAR* local_name, const { const size_t len = _wcslen(file->remote_name); - PathCchConvertStyleW(file->remote_name, len, PATH_STYLE_WINDOWS); + if (S_OK != PathCchConvertStyleW(file->remote_name, len, PATH_STYLE_WINDOWS)) + goto fail; } file->dwFileAttributes = fd.dwFileAttributes; diff --git a/winpr/libwinpr/utils/collections/MessagePipe.c b/winpr/libwinpr/utils/collections/MessagePipe.c index 0266f6e3a..62ef6b01f 100644 --- a/winpr/libwinpr/utils/collections/MessagePipe.c +++ b/winpr/libwinpr/utils/collections/MessagePipe.c @@ -18,6 +18,7 @@ */ #include +#include #include #include @@ -34,6 +35,7 @@ void MessagePipe_PostQuit(wMessagePipe* pipe, int nExitCode) { + WINPR_ASSERT(pipe); MessageQueue_PostQuit(pipe->In, nExitCode); MessageQueue_PostQuit(pipe->Out, nExitCode); } @@ -44,27 +46,23 @@ void MessagePipe_PostQuit(wMessagePipe* pipe, int nExitCode) wMessagePipe* MessagePipe_New(void) { - wMessagePipe* pipe = nullptr; - - pipe = (wMessagePipe*)malloc(sizeof(wMessagePipe)); + wMessagePipe* pipe = (wMessagePipe*)calloc(1, sizeof(wMessagePipe)); if (!pipe) return nullptr; pipe->In = MessageQueue_New(nullptr); if (!pipe->In) - goto error_in; + goto fail; pipe->Out = MessageQueue_New(nullptr); if (!pipe->Out) - goto error_out; + goto fail; return pipe; -error_out: - MessageQueue_Free(pipe->In); -error_in: - free(pipe); +fail: + MessagePipe_Free(pipe); return nullptr; }