From f794a8c7d49aa2914346552c9439fe3d7ba8e299 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 6 Sep 2021 16:11:48 +0200 Subject: [PATCH] Cleaned up remdesk --- channels/remdesk/client/remdesk_main.c | 107 ++++++++++++++++++++----- channels/remdesk/server/remdesk_main.c | 9 ++- include/freerdp/channels/remdesk.h | 3 +- 3 files changed, 92 insertions(+), 27 deletions(-) diff --git a/channels/remdesk/client/remdesk_main.c b/channels/remdesk/client/remdesk_main.c index a85ecdb69..144f5d61b 100644 --- a/channels/remdesk/client/remdesk_main.c +++ b/channels/remdesk/client/remdesk_main.c @@ -24,6 +24,7 @@ #endif #include +#include #include #include @@ -49,6 +50,7 @@ static UINT remdesk_virtual_channel_write(remdeskPlugin* remdesk, wStream* s) return CHANNEL_RC_INVALID_INSTANCE; } + WINPR_ASSERT(remdesk->channelEntryPoints.pVirtualChannelWriteEx); status = remdesk->channelEntryPoints.pVirtualChannelWriteEx( remdesk->InitHandle, remdesk->OpenHandle, Stream_Buffer(s), (UINT32)Stream_Length(s), s); @@ -71,7 +73,12 @@ static UINT remdesk_generate_expert_blob(remdeskPlugin* remdesk) char* name; char* pass; char* password; - rdpSettings* settings = remdesk->settings; + rdpSettings* settings; + + WINPR_ASSERT(remdesk); + + settings = remdesk->settings; + WINPR_ASSERT(settings); if (remdesk->ExpertBlob) return CHANNEL_RC_OK; @@ -133,6 +140,9 @@ static UINT remdesk_read_channel_header(wStream* s, REMDESK_CHANNEL_HEADER* head UINT32 ChannelNameLen; char* pChannelName = NULL; + WINPR_ASSERT(s); + WINPR_ASSERT(header); + if (Stream_GetRemainingLength(s) < 8) { WLog_ERR(TAG, "Not enough data!"); @@ -184,8 +194,10 @@ static UINT remdesk_write_channel_header(wStream* s, REMDESK_CHANNEL_HEADER* hea { int index; UINT32 ChannelNameLen; - WCHAR ChannelNameW[32]; - ZeroMemory(ChannelNameW, sizeof(ChannelNameW)); + WCHAR ChannelNameW[32] = { 0 }; + + WINPR_ASSERT(s); + WINPR_ASSERT(header); for (index = 0; index < 32; index++) { @@ -206,7 +218,10 @@ static UINT remdesk_write_channel_header(wStream* s, REMDESK_CHANNEL_HEADER* hea */ static UINT remdesk_write_ctl_header(wStream* s, REMDESK_CTL_HEADER* ctlHeader) { - remdesk_write_channel_header(s, (REMDESK_CHANNEL_HEADER*)ctlHeader); + WINPR_ASSERT(s); + WINPR_ASSERT(ctlHeader); + + remdesk_write_channel_header(s, &ctlHeader->ch); Stream_Write_UINT32(s, ctlHeader->msgType); /* msgType (4 bytes) */ return CHANNEL_RC_OK; } @@ -219,9 +234,12 @@ static UINT remdesk_write_ctl_header(wStream* s, REMDESK_CTL_HEADER* ctlHeader) static UINT remdesk_prepare_ctl_header(REMDESK_CTL_HEADER* ctlHeader, UINT32 msgType, UINT32 msgSize) { + WINPR_ASSERT(ctlHeader); + ctlHeader->msgType = msgType; - sprintf_s(ctlHeader->ChannelName, ARRAYSIZE(ctlHeader->ChannelName), REMDESK_CHANNEL_CTL_NAME); - ctlHeader->DataLength = 4 + msgSize; + sprintf_s(ctlHeader->ch.ChannelName, ARRAYSIZE(ctlHeader->ch.ChannelName), + REMDESK_CHANNEL_CTL_NAME); + ctlHeader->ch.DataLength = 4 + msgSize; return CHANNEL_RC_OK; } @@ -233,6 +251,10 @@ static UINT remdesk_prepare_ctl_header(REMDESK_CTL_HEADER* ctlHeader, UINT32 msg static UINT remdesk_recv_ctl_server_announce_pdu(remdeskPlugin* remdesk, wStream* s, REMDESK_CHANNEL_HEADER* header) { + WINPR_ASSERT(remdesk); + WINPR_ASSERT(s); + WINPR_ASSERT(header); + return CHANNEL_RC_OK; } @@ -247,6 +269,10 @@ static UINT remdesk_recv_ctl_version_info_pdu(remdeskPlugin* remdesk, wStream* s UINT32 versionMajor; UINT32 versionMinor; + WINPR_ASSERT(remdesk); + WINPR_ASSERT(s); + WINPR_ASSERT(header); + if (Stream_GetRemainingLength(s) < 8) { WLog_ERR(TAG, "Not enough data!"); @@ -276,10 +302,13 @@ static UINT remdesk_send_ctl_version_info_pdu(remdeskPlugin* remdesk) wStream* s; REMDESK_CTL_VERSION_INFO_PDU pdu; UINT error; + + WINPR_ASSERT(remdesk); + remdesk_prepare_ctl_header(&(pdu.ctlHeader), REMDESK_CTL_VERSIONINFO, 8); pdu.versionMajor = 1; pdu.versionMinor = 2; - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { @@ -308,6 +337,11 @@ static UINT remdesk_recv_ctl_result_pdu(remdeskPlugin* remdesk, wStream* s, { UINT32 result; + WINPR_ASSERT(remdesk); + WINPR_ASSERT(s); + WINPR_ASSERT(header); + WINPR_ASSERT(pResult); + if (Stream_GetRemainingLength(s) < 4) { WLog_ERR(TAG, "Not enough data!"); @@ -336,6 +370,8 @@ static UINT remdesk_send_ctl_authenticate_pdu(remdeskPlugin* remdesk) WCHAR* raConnectionStringW = NULL; REMDESK_CTL_AUTHENTICATE_PDU pdu = { 0 }; + WINPR_ASSERT(remdesk); + if ((error = remdesk_generate_expert_blob(remdesk))) { WLog_ERR(TAG, "remdesk_generate_expert_blob failed with error %" PRIu32 "", error); @@ -343,6 +379,7 @@ static UINT remdesk_send_ctl_authenticate_pdu(remdeskPlugin* remdesk) } pdu.expertBlob = remdesk->ExpertBlob; + WINPR_ASSERT(remdesk->settings); pdu.raConnectionString = remdesk->settings->RemoteAssistanceRCTicket; status = ConvertToUnicode(CP_UTF8, 0, pdu.raConnectionString, -1, &raConnectionStringW, 0); @@ -365,7 +402,7 @@ static UINT remdesk_send_ctl_authenticate_pdu(remdeskPlugin* remdesk) cbExpertBlobW = status * 2; remdesk_prepare_ctl_header(&(pdu.ctlHeader), REMDESK_CTL_AUTHENTICATE, cbRaConnectionStringW + cbExpertBlobW); - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { @@ -402,6 +439,10 @@ static UINT remdesk_send_ctl_remote_control_desktop_pdu(remdeskPlugin* remdesk) int cbRaConnectionStringW = 0; WCHAR* raConnectionStringW = NULL; REMDESK_CTL_REMOTE_CONTROL_DESKTOP_PDU pdu; + + WINPR_ASSERT(remdesk); + WINPR_ASSERT(remdesk->settings); + pdu.raConnectionString = remdesk->settings->RemoteAssistanceRCTicket; status = ConvertToUnicode(CP_UTF8, 0, pdu.raConnectionString, -1, &raConnectionStringW, 0); @@ -414,7 +455,7 @@ static UINT remdesk_send_ctl_remote_control_desktop_pdu(remdeskPlugin* remdesk) cbRaConnectionStringW = status * 2; remdesk_prepare_ctl_header(&(pdu.ctlHeader), REMDESK_CTL_REMOTE_CONTROL_DESKTOP, cbRaConnectionStringW); - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { @@ -450,6 +491,8 @@ static UINT remdesk_send_ctl_verify_password_pdu(remdeskPlugin* remdesk) WCHAR* expertBlobW = NULL; REMDESK_CTL_VERIFY_PASSWORD_PDU pdu; + WINPR_ASSERT(remdesk); + if ((error = remdesk_generate_expert_blob(remdesk))) { WLog_ERR(TAG, "remdesk_generate_expert_blob failed with error %" PRIu32 "!", error); @@ -467,7 +510,7 @@ static UINT remdesk_send_ctl_verify_password_pdu(remdeskPlugin* remdesk) cbExpertBlobW = status * 2; remdesk_prepare_ctl_header(&(pdu.ctlHeader), REMDESK_CTL_VERIFY_PASSWORD, cbExpertBlobW); - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { @@ -500,6 +543,8 @@ static UINT remdesk_send_ctl_expert_on_vista_pdu(remdeskPlugin* remdesk) wStream* s; REMDESK_CTL_EXPERT_ON_VISTA_PDU pdu; + WINPR_ASSERT(remdesk); + if ((error = remdesk_generate_expert_blob(remdesk))) { WLog_ERR(TAG, "remdesk_generate_expert_blob failed with error %" PRIu32 "!", error); @@ -510,7 +555,7 @@ static UINT remdesk_send_ctl_expert_on_vista_pdu(remdeskPlugin* remdesk) pdu.EncryptedPassword = remdesk->EncryptedPassStub; remdesk_prepare_ctl_header(&(pdu.ctlHeader), REMDESK_CTL_EXPERT_ON_VISTA, pdu.EncryptedPasswordLength); - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { @@ -535,6 +580,10 @@ static UINT remdesk_recv_ctl_pdu(remdeskPlugin* remdesk, wStream* s, REMDESK_CHA UINT32 msgType = 0; UINT32 result = 0; + WINPR_ASSERT(remdesk); + WINPR_ASSERT(s); + WINPR_ASSERT(header); + if (Stream_GetRemainingLength(s) < 4) { WLog_ERR(TAG, "Not enough data!"); @@ -659,6 +708,10 @@ static UINT remdesk_process_receive(remdeskPlugin* remdesk, wStream* s) { UINT status; REMDESK_CHANNEL_HEADER header; + + WINPR_ASSERT(remdesk); + WINPR_ASSERT(s); + #if 0 WLog_DBG(TAG, "RemdeskReceive: %"PRIuz"", Stream_GetRemainingLength(s)); winpr_HexDump(Stream_Pointer(s), Stream_GetRemainingLength(s)); @@ -698,6 +751,7 @@ static UINT remdesk_process_receive(remdeskPlugin* remdesk, wStream* s) static void remdesk_process_connect(remdeskPlugin* remdesk) { + WINPR_ASSERT(remdesk); remdesk->settings = (rdpSettings*)remdesk->channelEntryPoints.pExtendedData; } @@ -712,6 +766,8 @@ static UINT remdesk_virtual_channel_event_data_received(remdeskPlugin* remdesk, { wStream* data_in; + WINPR_ASSERT(remdesk); + if ((dataFlags & CHANNEL_FLAG_SUSPEND) || (dataFlags & CHANNEL_FLAG_RESUME)) { return CHANNEL_RC_OK; @@ -815,6 +871,9 @@ static DWORD WINAPI remdesk_virtual_channel_client_thread(LPVOID arg) wMessage message; remdeskPlugin* remdesk = (remdeskPlugin*)arg; UINT error = CHANNEL_RC_OK; + + WINPR_ASSERT(remdesk); + remdesk_process_connect(remdesk); while (1) @@ -908,21 +967,28 @@ static UINT remdesk_virtual_channel_event_disconnected(remdeskPlugin* remdesk) { UINT rc; + WINPR_ASSERT(remdesk); + if (remdesk->OpenHandle == 0) return CHANNEL_RC_OK; - if (MessageQueue_PostQuit(remdesk->queue, 0) && - (WaitForSingleObject(remdesk->thread, INFINITE) == WAIT_FAILED)) + if (remdesk->queue && remdesk->thread) { - rc = GetLastError(); - WLog_ERR(TAG, "WaitForSingleObject failed with error %" PRIu32 "", rc); - return rc; + if (MessageQueue_PostQuit(remdesk->queue, 0) && + (WaitForSingleObject(remdesk->thread, INFINITE) == WAIT_FAILED)) + { + rc = GetLastError(); + WLog_ERR(TAG, "WaitForSingleObject failed with error %" PRIu32 "", rc); + return rc; + } } MessageQueue_Free(remdesk->queue); CloseHandle(remdesk->thread); remdesk->queue = NULL; remdesk->thread = NULL; + + WINPR_ASSERT(remdesk->channelEntryPoints.pVirtualChannelCloseEx); rc = remdesk->channelEntryPoints.pVirtualChannelCloseEx(remdesk->InitHandle, remdesk->OpenHandle); @@ -934,17 +1000,16 @@ static UINT remdesk_virtual_channel_event_disconnected(remdeskPlugin* remdesk) remdesk->OpenHandle = 0; - if (remdesk->data_in) - { - Stream_Free(remdesk->data_in, TRUE); - remdesk->data_in = NULL; - } + Stream_Free(remdesk->data_in, TRUE); + remdesk->data_in = NULL; return rc; } static void remdesk_virtual_channel_event_terminated(remdeskPlugin* remdesk) { + WINPR_ASSERT(remdesk); + remdesk->InitHandle = 0; free(remdesk->context); free(remdesk); diff --git a/channels/remdesk/server/remdesk_main.c b/channels/remdesk/server/remdesk_main.c index 2be8056ca..fe32de5c4 100644 --- a/channels/remdesk/server/remdesk_main.c +++ b/channels/remdesk/server/remdesk_main.c @@ -148,8 +148,9 @@ static UINT remdesk_prepare_ctl_header(REMDESK_CTL_HEADER* ctlHeader, UINT32 msg UINT32 msgSize) { ctlHeader->msgType = msgType; - sprintf_s(ctlHeader->ChannelName, ARRAYSIZE(ctlHeader->ChannelName), REMDESK_CHANNEL_CTL_NAME); - ctlHeader->DataLength = 4 + msgSize; + sprintf_s(ctlHeader->ch.ChannelName, ARRAYSIZE(ctlHeader->ch.ChannelName), + REMDESK_CHANNEL_CTL_NAME); + ctlHeader->ch.DataLength = 4 + msgSize; return CHANNEL_RC_OK; } @@ -171,7 +172,7 @@ static UINT remdesk_send_ctl_result_pdu(RemdeskServerContext* context, UINT32 re return error; } - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { @@ -215,7 +216,7 @@ static UINT remdesk_send_ctl_version_info_pdu(RemdeskServerContext* context) pdu.versionMajor = 1; pdu.versionMinor = 2; - s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.DataLength); + s = Stream_New(NULL, REMDESK_CHANNEL_CTL_SIZE + pdu.ctlHeader.ch.DataLength); if (!s) { diff --git a/include/freerdp/channels/remdesk.h b/include/freerdp/channels/remdesk.h index a60f0e2d6..f68e194b1 100644 --- a/include/freerdp/channels/remdesk.h +++ b/include/freerdp/channels/remdesk.h @@ -86,8 +86,7 @@ typedef struct _REMDESK_CHANNEL_HEADER REMDESK_CHANNEL_HEADER; struct _REMDESK_CTL_HEADER { - UINT32 DataLength; - char ChannelName[32]; + REMDESK_CHANNEL_HEADER ch; UINT32 msgType; };