From 085f4e97059d77fd7ed9989f3f2e435952739daf Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 13 Jan 2026 16:39:45 +0100 Subject: [PATCH 1/4] [core,client] fix SVC channel init calls Ensure correct types are passed and returned. --- channels/client/CMakeLists.txt | 2 +- channels/client/tables.h | 2 +- libfreerdp/core/client.c | 77 ++++++++++++++++++++-------------- 3 files changed, 48 insertions(+), 33 deletions(-) diff --git a/channels/client/CMakeLists.txt b/channels/client/CMakeLists.txt index a5d596211..9d002b30e 100644 --- a/channels/client/CMakeLists.txt +++ b/channels/client/CMakeLists.txt @@ -41,7 +41,7 @@ foreach(STATIC_ENTRY ${CHANNEL_STATIC_CLIENT_ENTRIES}) if(${ENTRY} STREQUAL "VirtualChannelEntry") set(ENTRY_POINT_IMPORT "extern BOOL VCAPITYPE ${ENTRY_POINT_NAME}(PCHANNEL_ENTRY_POINTS);") elseif(${ENTRY} STREQUAL "VirtualChannelEntryEx") - set(ENTRY_POINT_IMPORT "extern BOOL VCAPITYPE ${ENTRY_POINT_NAME}(PCHANNEL_ENTRY_POINTS,PVOID);") + set(ENTRY_POINT_IMPORT "extern BOOL VCAPITYPE ${ENTRY_POINT_NAME}(PCHANNEL_ENTRY_POINTS_EX,PVOID);") elseif(${ENTRY} MATCHES "DVCPluginEntry$") set(ENTRY_POINT_IMPORT "extern UINT VCAPITYPE ${ENTRY_POINT_NAME}(IDRDYNVC_ENTRY_POINTS* pEntryPoints);") elseif(${ENTRY} MATCHES "DeviceServiceEntry$") diff --git a/channels/client/tables.h b/channels/client/tables.h index 7e26f5992..fdd4531d8 100644 --- a/channels/client/tables.h +++ b/channels/client/tables.h @@ -43,7 +43,7 @@ typedef struct static_entry_vc_fn_t entry; } STATIC_ENTRY_VC; -typedef BOOL(VCAPITYPE* static_entry_vcex_fn_t)(PCHANNEL_ENTRY_POINTS, PVOID); +typedef BOOL(VCAPITYPE* static_entry_vcex_fn_t)(PCHANNEL_ENTRY_POINTS_EX, PVOID); typedef struct { const char* name; diff --git a/libfreerdp/core/client.c b/libfreerdp/core/client.c index 9aa03e2b5..b01c36bb2 100644 --- a/libfreerdp/core/client.c +++ b/libfreerdp/core/client.c @@ -1385,8 +1385,6 @@ static BOOL freerdp_channels_is_loaded_ex(rdpChannels* channels, PVIRTUALCHANNEL int freerdp_channels_client_load(rdpChannels* channels, WINPR_ATTR_UNUSED rdpSettings* settings, PVIRTUALCHANNELENTRY entry, void* data) { - int status = 0; - CHANNEL_ENTRY_POINTS_FREERDP EntryPoints = { 0 }; CHANNEL_CLIENT_DATA* pChannelClientData = NULL; WINPR_ASSERT(channels); @@ -1409,19 +1407,29 @@ int freerdp_channels_client_load(rdpChannels* channels, WINPR_ATTR_UNUSED rdpSet pChannelClientData = &channels->clientDataList[channels->clientDataCount]; pChannelClientData->entry = entry; - EntryPoints.cbSize = sizeof(EntryPoints); - EntryPoints.protocolVersion = VIRTUAL_CHANNEL_VERSION_WIN2000; - EntryPoints.pVirtualChannelInit = FreeRDP_VirtualChannelInit; - EntryPoints.pVirtualChannelOpen = FreeRDP_VirtualChannelOpen; - EntryPoints.pVirtualChannelClose = FreeRDP_VirtualChannelClose; - EntryPoints.pVirtualChannelWrite = FreeRDP_VirtualChannelWrite; - EntryPoints.MagicNumber = FREERDP_CHANNEL_MAGIC_NUMBER; - EntryPoints.pExtendedData = data; - EntryPoints.context = channels->instance->context; + CHANNEL_ENTRY_POINTS_FREERDP EntryPoints = { .cbSize = sizeof(EntryPoints), + .protocolVersion = VIRTUAL_CHANNEL_VERSION_WIN2000, + .pVirtualChannelInit = FreeRDP_VirtualChannelInit, + .pVirtualChannelOpen = FreeRDP_VirtualChannelOpen, + .pVirtualChannelClose = + FreeRDP_VirtualChannelClose, + .pVirtualChannelWrite = + FreeRDP_VirtualChannelWrite, + .MagicNumber = FREERDP_CHANNEL_MAGIC_NUMBER, + .pExtendedData = data, + .context = channels->instance->context }; /* enable VirtualChannelInit */ + union + { + PCHANNEL_ENTRY_POINTS_FREERDP pfx; + PCHANNEL_ENTRY_POINTS px; + } ptr; + + ptr.pfx = &EntryPoints; + channels->can_call_init = TRUE; EnterCriticalSection(&channels->channelsLock); - status = pChannelClientData->entry((PCHANNEL_ENTRY_POINTS)&EntryPoints); + const BOOL status = pChannelClientData->entry(ptr.px); LeaveCriticalSection(&channels->channelsLock); /* disable MyVirtualChannelInit */ channels->can_call_init = FALSE; @@ -1438,12 +1446,6 @@ int freerdp_channels_client_load(rdpChannels* channels, WINPR_ATTR_UNUSED rdpSet int freerdp_channels_client_load_ex(rdpChannels* channels, WINPR_ATTR_UNUSED rdpSettings* settings, PVIRTUALCHANNELENTRYEX entryEx, void* data) { - int status = 0; - void* pInitHandle = NULL; - CHANNEL_ENTRY_POINTS_FREERDP_EX EntryPointsEx = { 0 }; - CHANNEL_INIT_DATA* pChannelInitData = NULL; - CHANNEL_CLIENT_DATA* pChannelClientData = NULL; - WINPR_ASSERT(channels); WINPR_ASSERT(channels->instance); WINPR_ASSERT(channels->instance->context); @@ -1461,24 +1463,37 @@ int freerdp_channels_client_load_ex(rdpChannels* channels, WINPR_ATTR_UNUSED rdp return 0; } - pChannelClientData = &channels->clientDataList[channels->clientDataCount]; + CHANNEL_CLIENT_DATA* pChannelClientData = &channels->clientDataList[channels->clientDataCount]; pChannelClientData->entryEx = entryEx; - pChannelInitData = &(channels->initDataList[channels->initDataCount++]); - pInitHandle = pChannelInitData; + + CHANNEL_INIT_DATA* pChannelInitData = &(channels->initDataList[channels->initDataCount++]); + void* pInitHandle = pChannelInitData; pChannelInitData->channels = channels; - EntryPointsEx.cbSize = sizeof(EntryPointsEx); - EntryPointsEx.protocolVersion = VIRTUAL_CHANNEL_VERSION_WIN2000; - EntryPointsEx.pVirtualChannelInitEx = FreeRDP_VirtualChannelInitEx; - EntryPointsEx.pVirtualChannelOpenEx = FreeRDP_VirtualChannelOpenEx; - EntryPointsEx.pVirtualChannelCloseEx = FreeRDP_VirtualChannelCloseEx; - EntryPointsEx.pVirtualChannelWriteEx = FreeRDP_VirtualChannelWriteEx; - EntryPointsEx.MagicNumber = FREERDP_CHANNEL_MAGIC_NUMBER; - EntryPointsEx.pExtendedData = data; - EntryPointsEx.context = channels->instance->context; + + CHANNEL_ENTRY_POINTS_FREERDP_EX EntryPointsEx = { + .cbSize = sizeof(EntryPointsEx), + .protocolVersion = VIRTUAL_CHANNEL_VERSION_WIN2000, + .pVirtualChannelInitEx = FreeRDP_VirtualChannelInitEx, + .pVirtualChannelOpenEx = FreeRDP_VirtualChannelOpenEx, + .pVirtualChannelCloseEx = FreeRDP_VirtualChannelCloseEx, + .pVirtualChannelWriteEx = FreeRDP_VirtualChannelWriteEx, + .MagicNumber = FREERDP_CHANNEL_MAGIC_NUMBER, + .pExtendedData = data, + .context = channels->instance->context + }; + + union + { + PCHANNEL_ENTRY_POINTS_FREERDP_EX pfx; + PCHANNEL_ENTRY_POINTS_EX px; + } ptr; + + ptr.pfx = &EntryPointsEx; + /* enable VirtualChannelInit */ channels->can_call_init = TRUE; EnterCriticalSection(&channels->channelsLock); - status = pChannelClientData->entryEx((PCHANNEL_ENTRY_POINTS_EX)&EntryPointsEx, pInitHandle); + const BOOL status = pChannelClientData->entryEx(ptr.px, pInitHandle); LeaveCriticalSection(&channels->channelsLock); /* disable MyVirtualChannelInit */ channels->can_call_init = FALSE; From d60045ae1a8b269d800e6a1bdeb942291b43f0a1 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 13 Jan 2026 16:43:59 +0100 Subject: [PATCH 2/4] [chanels,various] fix VirtualChannelEntryEx parameter types --- channels/cliprdr/client/cliprdr_main.c | 2 +- channels/rail/client/rail_main.c | 2 +- channels/rdp2tcp/client/rdp2tcp_main.c | 2 +- channels/rdpdr/client/rdpdr_main.c | 13 +++++-------- channels/rdpsnd/client/rdpsnd_main.c | 4 ++-- channels/remdesk/client/remdesk_main.c | 2 +- 6 files changed, 11 insertions(+), 14 deletions(-) diff --git a/channels/cliprdr/client/cliprdr_main.c b/channels/cliprdr/client/cliprdr_main.c index fc88dbcf0..f4ce710d9 100644 --- a/channels/cliprdr/client/cliprdr_main.c +++ b/channels/cliprdr/client/cliprdr_main.c @@ -1134,7 +1134,7 @@ static VOID VCAPITYPE cliprdr_virtual_channel_init_event_ex(LPVOID lpUserParam, /* cliprdr is always built-in */ #define VirtualChannelEntryEx cliprdr_VirtualChannelEntryEx -FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, +FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS_EX pEntryPoints, PVOID pInitHandle)) { UINT rc = 0; diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index 256336481..41fa9c8d3 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -676,7 +676,7 @@ static VOID VCAPITYPE rail_virtual_channel_init_event_ex(LPVOID lpUserParam, LPV /* rail is always built-in */ #define VirtualChannelEntryEx rail_VirtualChannelEntryEx -FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, +FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS_EX pEntryPoints, PVOID pInitHandle)) { UINT rc = 0; diff --git a/channels/rdp2tcp/client/rdp2tcp_main.c b/channels/rdp2tcp/client/rdp2tcp_main.c index 69ad8564d..826f152c1 100644 --- a/channels/rdp2tcp/client/rdp2tcp_main.c +++ b/channels/rdp2tcp/client/rdp2tcp_main.c @@ -289,7 +289,7 @@ static VOID VCAPITYPE VirtualChannelInitEventEx(LPVOID lpUserParam, LPVOID pInit } #define VirtualChannelEntryEx rdp2tcp_VirtualChannelEntryEx -FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, +FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS_EX pEntryPoints, PVOID pInitHandle)) { CHANNEL_ENTRY_POINTS_FREERDP_EX* pEntryPointsEx = diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index cc84c8ac8..caf2d6371 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -2420,17 +2420,13 @@ static VOID VCAPITYPE rdpdr_virtual_channel_init_event_ex(LPVOID lpUserParam, LP /* rdpdr is always built-in */ #define VirtualChannelEntryEx rdpdr_VirtualChannelEntryEx -FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, +FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS_EX pEntryPoints, PVOID pInitHandle)) { - UINT rc = 0; - rdpdrPlugin* rdpdr = NULL; - CHANNEL_ENTRY_POINTS_FREERDP_EX* pEntryPointsEx = NULL; - WINPR_ASSERT(pEntryPoints); WINPR_ASSERT(pInitHandle); - rdpdr = (rdpdrPlugin*)calloc(1, sizeof(rdpdrPlugin)); + rdpdrPlugin* rdpdr = (rdpdrPlugin*)calloc(1, sizeof(rdpdrPlugin)); if (!rdpdr) { @@ -2463,7 +2459,8 @@ FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS p (void)sprintf_s(rdpdr->channelDef.name, ARRAYSIZE(rdpdr->channelDef.name), RDPDR_SVC_CHANNEL_NAME); rdpdr->sequenceId = 0; - pEntryPointsEx = (CHANNEL_ENTRY_POINTS_FREERDP_EX*)pEntryPoints; + CHANNEL_ENTRY_POINTS_FREERDP_EX* pEntryPointsEx = + (CHANNEL_ENTRY_POINTS_FREERDP_EX*)pEntryPoints; if ((pEntryPointsEx->cbSize >= sizeof(CHANNEL_ENTRY_POINTS_FREERDP_EX)) && (pEntryPointsEx->MagicNumber == FREERDP_CHANNEL_MAGIC_NUMBER)) @@ -2476,7 +2473,7 @@ FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS p CopyMemory(&(rdpdr->channelEntryPoints), pEntryPoints, sizeof(CHANNEL_ENTRY_POINTS_FREERDP_EX)); rdpdr->InitHandle = pInitHandle; - rc = rdpdr->channelEntryPoints.pVirtualChannelInitEx( + const UINT rc = rdpdr->channelEntryPoints.pVirtualChannelInitEx( rdpdr, &rdpdr->context, pInitHandle, &rdpdr->channelDef, 1, VIRTUAL_CHANNEL_VERSION_WIN2000, rdpdr_virtual_channel_init_event_ex); diff --git a/channels/rdpsnd/client/rdpsnd_main.c b/channels/rdpsnd/client/rdpsnd_main.c index af313f8d7..49c763a87 100644 --- a/channels/rdpsnd/client/rdpsnd_main.c +++ b/channels/rdpsnd/client/rdpsnd_main.c @@ -1567,8 +1567,8 @@ fail: return NULL; } /* rdpsnd is always built-in */ -FREERDP_ENTRY_POINT(BOOL VCAPITYPE rdpsnd_VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, - PVOID pInitHandle)) +FREERDP_ENTRY_POINT(BOOL VCAPITYPE rdpsnd_VirtualChannelEntryEx( + PCHANNEL_ENTRY_POINTS_EX pEntryPoints, PVOID pInitHandle)) { UINT rc = 0; rdpsndPlugin* rdpsnd = NULL; diff --git a/channels/remdesk/client/remdesk_main.c b/channels/remdesk/client/remdesk_main.c index e1a75cb9b..4127c789e 100644 --- a/channels/remdesk/client/remdesk_main.c +++ b/channels/remdesk/client/remdesk_main.c @@ -987,7 +987,7 @@ static VOID VCAPITYPE remdesk_virtual_channel_init_event_ex(LPVOID lpUserParam, /* remdesk is always built-in */ #define VirtualChannelEntryEx remdesk_VirtualChannelEntryEx -FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, +FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS_EX pEntryPoints, PVOID pInitHandle)) { UINT rc = 0; From 480f9491de6f9081e6cfc6fa52cf683158f1b411 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 13 Jan 2026 16:20:16 +0100 Subject: [PATCH 3/4] [winpr,pipe] lpOverlapped checks --- winpr/libwinpr/pipe/pipe.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/winpr/libwinpr/pipe/pipe.c b/winpr/libwinpr/pipe/pipe.c index 69b12744b..f8392b18e 100644 --- a/winpr/libwinpr/pipe/pipe.c +++ b/winpr/libwinpr/pipe/pipe.c @@ -251,17 +251,9 @@ BOOL NamedPipeRead(PVOID Object, LPVOID lpBuffer, DWORD nNumberOfBytesToRead, LPDWORD lpNumberOfBytesRead, LPOVERLAPPED lpOverlapped) { SSIZE_T io_status = 0; - WINPR_NAMED_PIPE* pipe = NULL; BOOL status = TRUE; - if (lpOverlapped) - { - WLog_ERR(TAG, "WinPR does not support the lpOverlapped parameter"); - SetLastError(ERROR_NOT_SUPPORTED); - return FALSE; - } - - pipe = (WINPR_NAMED_PIPE*)Object; + WINPR_NAMED_PIPE* pipe = (WINPR_NAMED_PIPE*)Object; if (!(pipe->dwFlagsAndAttributes & FILE_FLAG_OVERLAPPED)) { @@ -301,7 +293,11 @@ BOOL NamedPipeRead(PVOID Object, LPVOID lpBuffer, DWORD nNumberOfBytesToRead, { /* Overlapped I/O */ if (!lpOverlapped) + { + WLog_ERR(TAG, "%s requires lpOverlapped != NULL as FILE_FLAG_OVERLAPPED is set"); + SetLastError(ERROR_NOT_SUPPORTED); return FALSE; + } if (pipe->clientfd == -1) return FALSE; From 5c940eda5104b440cefdbaa0bc432155c154074a Mon Sep 17 00:00:00 2001 From: David Lechevalier Date: Tue, 13 Jan 2026 16:40:14 +0100 Subject: [PATCH 4/4] [printer] Compute printer configuration file using sha256 When printer name are long, using only base64 will create a long file which can cause the issue ERROR_FILENAME_EXCED_RANGE on Windows. --- channels/printer/client/printer_main.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/channels/printer/client/printer_main.c b/channels/printer/client/printer_main.c index 439edf24b..2240e594c 100644 --- a/channels/printer/client/printer_main.c +++ b/channels/printer/client/printer_main.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -77,12 +78,29 @@ typedef enum static const char* filemap[] = { "PortDosName", "PnPName", "DriverName", "CachedPrinterConfigData" }; +WINPR_ATTR_MALLOC(free, 1) +static char* get_printer_hash(const WCHAR* name, size_t length) +{ + BYTE hash[WINPR_SHA256_DIGEST_LENGTH] = { 0 }; + + if (!winpr_Digest(WINPR_MD_SHA256, name, length, hash, sizeof(hash))) + return NULL; + + return winpr_BinToHexString(hash, sizeof(hash), FALSE); +} + +WINPR_ATTR_MALLOC(free, 1) static char* get_printer_config_path(const rdpSettings* settings, const WCHAR* name, size_t length) { + char* config = NULL; const char* path = freerdp_settings_get_string(settings, FreeRDP_ConfigPath); char* dir = GetCombinedPath(path, "printers"); - char* bname = crypto_base64_encode((const BYTE*)name, length); - char* config = GetCombinedPath(dir, bname); + if (!dir) + return NULL; + char* bname = get_printer_hash(name, length); + if (!bname) + goto fail; + config = GetCombinedPath(dir, bname); if (config && !winpr_PathFileExists(config)) { @@ -93,6 +111,7 @@ static char* get_printer_config_path(const rdpSettings* settings, const WCHAR* n } } +fail: free(dir); free(bname); return config; @@ -101,6 +120,9 @@ static char* get_printer_config_path(const rdpSettings* settings, const WCHAR* n static BOOL printer_write_setting(const char* path, prn_conf_t type, const void* data, size_t length) { + if (!path) + return FALSE; + DWORD written = 0; BOOL rc = FALSE; HANDLE file = NULL;