From 17163d373887a66714a64d8c1731a85468da9a27 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 2 Mar 2026 13:17:00 +0100 Subject: [PATCH 1/8] [winpr,collections] fix PubSub_OnEvent return checks * proper return checks on use * fix return on invalid input arguments * fix return on no event registered --- channels/rdpgfx/client/rdpgfx_main.c | 6 +- client/Mac/MRDPView.m | 7 ++- client/Windows/wf_client.c | 4 +- client/Windows/wf_event.c | 3 +- client/X11/xf_client.c | 13 +++-- client/X11/xf_event.c | 5 +- client/X11/xf_input.c | 46 ++++++++++----- client/X11/xf_input.h | 5 ++ client/X11/xf_keyboard.c | 6 +- libfreerdp/core/client.c | 68 +++++++++++++++++++---- libfreerdp/core/connection.c | 6 +- libfreerdp/core/freerdp.c | 6 +- libfreerdp/core/gateway/rpc_client.c | 3 +- libfreerdp/core/rdp.c | 3 +- winpr/libwinpr/utils/collections/PubSub.c | 23 ++++---- winpr/libwinpr/utils/test/TestPubSub.c | 6 +- 16 files changed, 152 insertions(+), 58 deletions(-) diff --git a/channels/rdpgfx/client/rdpgfx_main.c b/channels/rdpgfx/client/rdpgfx_main.c index 26b29d1b2..0c42b5932 100644 --- a/channels/rdpgfx/client/rdpgfx_main.c +++ b/channels/rdpgfx/client/rdpgfx_main.c @@ -592,12 +592,14 @@ static UINT rdpgfx_recv_reset_graphics_pdu(GENERIC_CHANNEL_CALLBACK* callback, w error); } + free(pdu.monitorDefArray); + /* some listeners may be interested (namely the display channel) */ EventArgsInit(&graphicsReset, "libfreerdp"); graphicsReset.width = pdu.width; graphicsReset.height = pdu.height; - PubSub_OnGraphicsReset(gfx->rdpcontext->pubSub, gfx->rdpcontext, &graphicsReset); - free(pdu.monitorDefArray); + if (PubSub_OnGraphicsReset(gfx->rdpcontext->pubSub, gfx->rdpcontext, &graphicsReset) < 0) + return ERROR_INTERNAL_ERROR; return error; } diff --git a/client/Mac/MRDPView.m b/client/Mac/MRDPView.m index 2d9a20d5c..e82f50b9c 100644 --- a/client/Mac/MRDPView.m +++ b/client/Mac/MRDPView.m @@ -91,7 +91,9 @@ static CGContextRef mac_create_bitmap_context(rdpContext *context); EventArgsInit(&e, "mfreerdp"); e.embed = TRUE; e.handle = (void *)self; - PubSub_OnEmbedWindow(context->pubSub, context, &e); + if (PubSub_OnEmbedWindow(context->pubSub, context, &e) < 0) + return -1; + NSScreen *screen = [[NSScreen screens] objectAtIndex:0]; NSRect screenFrame = [screen frame]; @@ -1495,7 +1497,8 @@ BOOL mac_desktop_resize(rdpContext *context) EventArgsInit(&e, "mfreerdp"); e.width = freerdp_settings_get_uint32(settings, FreeRDP_DesktopWidth); e.height = freerdp_settings_get_uint32(settings, FreeRDP_DesktopHeight); - PubSub_OnResizeWindow(context->pubSub, context, &e); + if (PubSub_OnResizeWindow(context->pubSub, context, &e) < 0) + return FALSE; return TRUE; } diff --git a/client/Windows/wf_client.c b/client/Windows/wf_client.c index e2459b9d6..6ea87c84b 100644 --- a/client/Windows/wf_client.c +++ b/client/Windows/wf_client.c @@ -477,7 +477,9 @@ static BOOL wf_post_connect(freerdp* instance) EventArgsInit(&e, "wfreerdp"); e.embed = FALSE; e.handle = (void*)wfc->hwnd; - PubSub_OnEmbedWindow(context->pubSub, context, &e); + if (PubSub_OnEmbedWindow(context->pubSub, context, &e) < 0) + return FALSE; + #ifdef WITH_PROGRESS_BAR if (wfc->taskBarList) { diff --git a/client/Windows/wf_event.c b/client/Windows/wf_event.c index ed0fc3840..0bda81c38 100644 --- a/client/Windows/wf_event.c +++ b/client/Windows/wf_event.c @@ -927,8 +927,7 @@ static BOOL wf_pub_mouse_event(wfContext* wfc, UINT16 flags, UINT16 x, UINT16 y) eventArgs.flags = flags; eventArgs.x = x; eventArgs.y = y; - PubSub_OnMouseEvent(wfc->common.context.pubSub, &wfc->common.context, &eventArgs); - return TRUE; + return PubSub_OnMouseEvent(wfc->common.context.pubSub, &wfc->common.context, &eventArgs) >= 0; } static BOOL wf_scale_mouse_event(wfContext* wfc, UINT16 flags, INT32 x, INT32 y) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 17d4e21ce..e4cf2be68 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -826,7 +826,10 @@ void xf_toggle_fullscreen(xfContext* xfc) xf_SetWindowFullscreen(xfc, xfc->window, xfc->fullscreen); EventArgsInit(&e, "xfreerdp"); e.state = xfc->fullscreen ? FREERDP_WINDOW_STATE_FULLSCREEN : 0; - PubSub_OnWindowStateChange(context->pubSub, context, &e); + if (PubSub_OnWindowStateChange(context->pubSub, context, &e) < 0) + { + WLog_Print(xfc->log, WLOG_ERROR, "PubSub_OnWindowStateChange failed"); + } } void xf_minimize(xfContext* xfc) @@ -845,7 +848,10 @@ void xf_minimize(xfContext* xfc) xf_SetWindowMinimized(xfc, xfc->window); e.state = xfc->fullscreen ? FREERDP_WINDOW_STATE_FULLSCREEN : 0; - PubSub_OnWindowStateChange(context->pubSub, context, &e); + if (PubSub_OnWindowStateChange(context->pubSub, context, &e) < 0) + { + WLog_Print(xfc->log, WLOG_ERROR, "PubSub_OnWindowStateChange failed"); + } } void xf_lock_x11_(xfContext* xfc, WINPR_ATTR_UNUSED const char* fkt) @@ -1471,8 +1477,7 @@ static BOOL xf_post_connect(freerdp* instance) WINPR_ASSERTING_INT_CAST(int, freerdp_settings_get_uint32(settings, FreeRDP_DesktopWidth)); e.height = WINPR_ASSERTING_INT_CAST(int, freerdp_settings_get_uint32(settings, FreeRDP_DesktopHeight)); - PubSub_OnResizeWindow(context->pubSub, xfc, &e); - return TRUE; + return PubSub_OnResizeWindow(context->pubSub, xfc, &e) >= 0; } static void xf_post_disconnect(freerdp* instance) diff --git a/client/X11/xf_event.c b/client/X11/xf_event.c index 1a05b42a9..833b07cb9 100644 --- a/client/X11/xf_event.c +++ b/client/X11/xf_event.c @@ -1369,7 +1369,10 @@ BOOL xf_event_process(freerdp* instance, const XEvent* event) xf_cliprdr_handle_xevent(xfc, event); if (!xf_floatbar_check_event(floatbar, event) && !xf_floatbar_is_locked(floatbar)) - xf_input_handle_event(xfc, event); + { + if (xf_input_handle_event(xfc, event) < 0) + return FALSE; + } LogDynAndXSync(xfc->log, xfc->display, FALSE); return status; diff --git a/client/X11/xf_input.c b/client/X11/xf_input.c index dba6e0f5f..297b9ecd9 100644 --- a/client/X11/xf_input.c +++ b/client/X11/xf_input.c @@ -321,7 +321,8 @@ static void xf_input_save_last_event(xfContext* xfc, const XGenericEventCookie* xfc->lastEvent.event_y = event->event_y; } -static void xf_input_detect_pan(xfContext* xfc) +WINPR_ATTR_NODISCARD +static BOOL xf_input_detect_pan(xfContext* xfc) { WINPR_ASSERT(xfc); rdpContext* ctx = &xfc->common.context; @@ -329,7 +330,7 @@ static void xf_input_detect_pan(xfContext* xfc) if (xfc->active_contacts != 2) { - return; + return TRUE; } const double dx[] = { xfc->contacts[0].pos_x - xfc->contacts[0].last_x, @@ -352,7 +353,8 @@ static void xf_input_detect_pan(xfContext* xfc) EventArgsInit(&e, "xfreerdp"); e.dx = 5; e.dy = 0; - PubSub_OnPanningChange(ctx->pubSub, xfc, &e); + if (PubSub_OnPanningChange(ctx->pubSub, xfc, &e) < 0) + return FALSE; } xfc->px_vector = 0; xfc->py_vector = 0; @@ -365,7 +367,8 @@ static void xf_input_detect_pan(xfContext* xfc) EventArgsInit(&e, "xfreerdp"); e.dx = -5; e.dy = 0; - PubSub_OnPanningChange(ctx->pubSub, xfc, &e); + if (PubSub_OnPanningChange(ctx->pubSub, xfc, &e) < 0) + return FALSE; } xfc->px_vector = 0; xfc->py_vector = 0; @@ -382,7 +385,8 @@ static void xf_input_detect_pan(xfContext* xfc) EventArgsInit(&e, "xfreerdp"); e.dx = 0; e.dy = 5; - PubSub_OnPanningChange(ctx->pubSub, xfc, &e); + if (PubSub_OnPanningChange(ctx->pubSub, xfc, &e) < 0) + return FALSE; } xfc->py_vector = 0; xfc->px_vector = 0; @@ -395,16 +399,19 @@ static void xf_input_detect_pan(xfContext* xfc) EventArgsInit(&e, "xfreerdp"); e.dx = 0; e.dy = -5; - PubSub_OnPanningChange(ctx->pubSub, xfc, &e); + if (PubSub_OnPanningChange(ctx->pubSub, xfc, &e) < 0) + return FALSE; } xfc->py_vector = 0; xfc->px_vector = 0; xfc->z_vector = 0; } } + return TRUE; } -static void xf_input_detect_pinch(xfContext* xfc) +WINPR_ATTR_NODISCARD +static BOOL xf_input_detect_pinch(xfContext* xfc) { ZoomingChangeEventArgs e = WINPR_C_ARRAY_INIT; @@ -415,7 +422,7 @@ static void xf_input_detect_pinch(xfContext* xfc) if (xfc->active_contacts != 2) { xfc->firstDist = -1.0; - return; + return TRUE; } /* first calculate the distance */ @@ -449,7 +456,8 @@ static void xf_input_detect_pinch(xfContext* xfc) { EventArgsInit(&e, "xfreerdp"); e.dx = e.dy = -10; - PubSub_OnZoomingChange(ctx->pubSub, xfc, &e); + if (PubSub_OnZoomingChange(ctx->pubSub, xfc, &e) < 0) + return FALSE; xfc->z_vector = 0; xfc->px_vector = 0; xfc->py_vector = 0; @@ -459,12 +467,14 @@ static void xf_input_detect_pinch(xfContext* xfc) { EventArgsInit(&e, "xfreerdp"); e.dx = e.dy = 10; - PubSub_OnZoomingChange(ctx->pubSub, xfc, &e); + if (PubSub_OnZoomingChange(ctx->pubSub, xfc, &e) < 0) + return FALSE; xfc->z_vector = 0; xfc->px_vector = 0; xfc->py_vector = 0; } } + return TRUE; } static void xf_input_touch_begin(xfContext* xfc, const XIDeviceEvent* event) @@ -484,7 +494,8 @@ static void xf_input_touch_begin(xfContext* xfc, const XIDeviceEvent* event) } } -static void xf_input_touch_update(xfContext* xfc, const XIDeviceEvent* event) +WINPR_ATTR_NODISCARD +static BOOL xf_input_touch_update(xfContext* xfc, const XIDeviceEvent* event) { WINPR_ASSERT(xfc); WINPR_ASSERT(event); @@ -498,11 +509,15 @@ static void xf_input_touch_update(xfContext* xfc, const XIDeviceEvent* event) xfc->contacts[i].last_y = xfc->contacts[i].pos_y; xfc->contacts[i].pos_x = event->event_x; xfc->contacts[i].pos_y = event->event_y; - xf_input_detect_pinch(xfc); - xf_input_detect_pan(xfc); + if (!xf_input_detect_pinch(xfc)) + return FALSE; + if (!xf_input_detect_pan(xfc)) + return FALSE; break; } } + + return TRUE; } static void xf_input_touch_end(xfContext* xfc, const XIDeviceEvent* event) @@ -543,7 +558,10 @@ static int xf_input_handle_event_local(xfContext* xfc, const XEvent* event) case XI_TouchUpdate: if (xf_input_is_duplicate(xfc, cookie.cc) == FALSE) - xf_input_touch_update(xfc, cookie.cc->data); + { + if (!xf_input_touch_update(xfc, cookie.cc->data)) + return -1; + } xf_input_save_last_event(xfc, cookie.cc); break; diff --git a/client/X11/xf_input.h b/client/X11/xf_input.h index 49a8e1959..1b342a2b6 100644 --- a/client/X11/xf_input.h +++ b/client/X11/xf_input.h @@ -27,8 +27,13 @@ #include #endif +WINPR_ATTR_NODISCARD int xf_input_init(xfContext* xfc, Window window); + +WINPR_ATTR_NODISCARD int xf_input_handle_event(xfContext* xfc, const XEvent* event); + +WINPR_ATTR_NODISCARD bool xf_use_rel_mouse(xfContext* xfc); #endif /* FREERDP_CLIENT_X11_INPUT_H */ diff --git a/client/X11/xf_keyboard.c b/client/X11/xf_keyboard.c index e88c6ab58..86a21c221 100644 --- a/client/X11/xf_keyboard.c +++ b/client/X11/xf_keyboard.c @@ -1466,7 +1466,8 @@ BOOL xf_keyboard_handle_special_keys(xfContext* xfc, KeySym keysym) EventArgsInit(&e, "xfreerdp"); e.dx = pdx; e.dy = pdy; - PubSub_OnPanningChange(ctx->pubSub, xfc, &e); + if (PubSub_OnPanningChange(ctx->pubSub, xfc, &e)<0) + return FALSE; return TRUE; } @@ -1476,7 +1477,8 @@ BOOL xf_keyboard_handle_special_keys(xfContext* xfc, KeySym keysym) EventArgsInit(&e, "xfreerdp"); e.dx = zdx; e.dy = zdy; - PubSub_OnZoomingChange(ctx->pubSub, xfc, &e); + if (PubSub_OnZoomingChange(ctx->pubSub, xfc, &e)<0) + return FALSE; return TRUE; } } diff --git a/libfreerdp/core/client.c b/libfreerdp/core/client.c index 1dfacd460..0e695f5fe 100644 --- a/libfreerdp/core/client.c +++ b/libfreerdp/core/client.c @@ -223,12 +223,21 @@ static UINT freerdp_drdynvc_on_channel_connected(DrdynvcClientContext* context, { UINT status = CHANNEL_RC_OK; ChannelConnectedEventArgs e = WINPR_C_ARRAY_INIT; + + WINPR_ASSERT(context); + rdpChannels* channels = (rdpChannels*)context->custom; + WINPR_ASSERT(channels); + freerdp* instance = channels->instance; + WINPR_ASSERT(instance); + WINPR_ASSERT(instance->context); + EventArgsInit(&e, "freerdp"); e.name = name; e.pInterface = pInterface; - PubSub_OnChannelConnected(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelConnected(instance->context->pubSub, instance->context, &e) < 0) + return ERROR_INTERNAL_ERROR; return status; } @@ -242,12 +251,20 @@ static UINT freerdp_drdynvc_on_channel_disconnected(DrdynvcClientContext* contex { UINT status = CHANNEL_RC_OK; ChannelDisconnectedEventArgs e = WINPR_C_ARRAY_INIT; + + WINPR_ASSERT(context); rdpChannels* channels = (rdpChannels*)context->custom; + WINPR_ASSERT(channels); + freerdp* instance = channels->instance; + WINPR_ASSERT(instance); + WINPR_ASSERT(instance->context); + EventArgsInit(&e, "freerdp"); e.name = name; e.pInterface = pInterface; - PubSub_OnChannelDisconnected(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelDisconnected(instance->context->pubSub, instance->context, &e) < 0) + return ERROR_INTERNAL_ERROR; return status; } @@ -256,12 +273,20 @@ static UINT freerdp_drdynvc_on_channel_attached(DrdynvcClientContext* context, c { UINT status = CHANNEL_RC_OK; ChannelAttachedEventArgs e = WINPR_C_ARRAY_INIT; + + WINPR_ASSERT(context); rdpChannels* channels = (rdpChannels*)context->custom; + WINPR_ASSERT(channels); + freerdp* instance = channels->instance; + WINPR_ASSERT(instance); + WINPR_ASSERT(instance->context); + EventArgsInit(&e, "freerdp"); e.name = name; e.pInterface = pInterface; - PubSub_OnChannelAttached(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelAttached(instance->context->pubSub, instance->context, &e) < 0) + return ERROR_INTERNAL_ERROR; return status; } @@ -270,12 +295,20 @@ static UINT freerdp_drdynvc_on_channel_detached(DrdynvcClientContext* context, c { UINT status = CHANNEL_RC_OK; ChannelDetachedEventArgs e = WINPR_C_ARRAY_INIT; + + WINPR_ASSERT(context); rdpChannels* channels = (rdpChannels*)context->custom; + WINPR_ASSERT(channels); + freerdp* instance = channels->instance; + WINPR_ASSERT(instance); + WINPR_ASSERT(instance->context); + EventArgsInit(&e, "freerdp"); e.name = name; e.pInterface = pInterface; - PubSub_OnChannelDetached(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelDetached(instance->context->pubSub, instance->context, &e) < 0) + return ERROR_INTERNAL_ERROR; return status; } @@ -363,14 +396,19 @@ UINT freerdp_channels_attach(freerdp* instance) CHANNEL_EVENT_ATTACHED, cnv.pv, (UINT)hostnameLength); } - if (getChannelError(instance->context) != CHANNEL_RC_OK) + error = getChannelError(instance->context); + if (error != CHANNEL_RC_OK) goto fail; pChannelOpenData = &channels->openDataList[index]; EventArgsInit(&e, "freerdp"); e.name = pChannelOpenData->name; e.pInterface = pChannelOpenData->pInterface; - PubSub_OnChannelAttached(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelAttached(instance->context->pubSub, instance->context, &e) < 0) + { + error = ERROR_INTERNAL_ERROR; + goto fail; + } } fail: @@ -426,14 +464,19 @@ UINT freerdp_channels_detach(freerdp* instance) CHANNEL_EVENT_DETACHED, cnv.pv, (UINT)hostnameLength); } - if (getChannelError(context) != CHANNEL_RC_OK) + error = getChannelError(context); + if (error != CHANNEL_RC_OK) goto fail; pChannelOpenData = &channels->openDataList[index]; EventArgsInit(&e, "freerdp"); e.name = pChannelOpenData->name; e.pInterface = pChannelOpenData->pInterface; - PubSub_OnChannelDetached(context->pubSub, context, &e); + if (PubSub_OnChannelDetached(context->pubSub, context, &e) < 0) + { + error = ERROR_INTERNAL_ERROR; + goto fail; + } } fail: @@ -495,7 +538,11 @@ UINT freerdp_channels_post_connect(rdpChannels* channels, freerdp* instance) EventArgsInit(&e, "freerdp"); e.name = pChannelOpenData->name; e.pInterface = pChannelOpenData->pInterface; - PubSub_OnChannelConnected(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelConnected(instance->context->pubSub, instance->context, &e) < 0) + { + error = ERROR_INTERNAL_ERROR; + goto fail; + } } channels->drdynvc = (DrdynvcClientContext*)freerdp_channels_get_static_channel_interface( @@ -889,7 +936,8 @@ UINT freerdp_channels_disconnect(rdpChannels* channels, freerdp* instance) EventArgsInit(&e, "freerdp"); e.name = pChannelOpenData->name; e.pInterface = pChannelOpenData->pInterface; - PubSub_OnChannelDisconnected(instance->context->pubSub, instance->context, &e); + if (PubSub_OnChannelDisconnected(instance->context->pubSub, instance->context, &e) < 0) + error = ERROR_INTERNAL_ERROR; } channels->connected = FALSE; diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index 3a8032e49..c71325017 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -1419,7 +1419,8 @@ BOOL rdp_client_transition_to_state(rdpRdp* rdp, CONNECTION_STATE state) EventArgsInit(&activatedEvent, "libfreerdp"); activatedEvent.firstActivation = !rdp_finalize_is_flag_set(rdp, FINALIZE_DEACTIVATE_REACTIVATE); - PubSub_OnActivated(rdp->pubSub, context, &activatedEvent); + if (PubSub_OnActivated(rdp->pubSub, context, &activatedEvent) < 0) + return FALSE; } break; @@ -1434,7 +1435,8 @@ BOOL rdp_client_transition_to_state(rdpRdp* rdp, CONNECTION_STATE state) EventArgsInit(&stateEvent, "libfreerdp"); stateEvent.state = WINPR_ASSERTING_INT_CAST(int32_t, rdp_get_state(rdp)); stateEvent.active = rdp_is_active_state(rdp); - PubSub_OnConnectionStateChange(rdp->pubSub, context, &stateEvent); + if (PubSub_OnConnectionStateChange(rdp->pubSub, context, &stateEvent) < 0) + return FALSE; } return TRUE; diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index 2e4fe80b8..7367e8e5f 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -303,7 +303,8 @@ BOOL freerdp_connect(freerdp* instance) freerdp_connect_finally: EventArgsInit(&e, "freerdp"); e.result = status ? 0 : -1; - PubSub_OnConnectionResult(rdp->pubSub, instance->context, &e); + if (PubSub_OnConnectionResult(rdp->pubSub, instance->context, &e) < 0) + return FALSE; if (!status) freerdp_disconnect(instance); @@ -373,7 +374,8 @@ BOOL freerdp_check_fds(freerdp* instance) WLog_Print(context->log, WLOG_DEBUG, "rdp_check_fds() - %i", status); EventArgsInit(&e, "freerdp"); e.code = 0; - PubSub_OnTerminate(rdp->pubSub, context, &e); + if (PubSub_OnTerminate(rdp->pubSub, context, &e) < 0) + return FALSE; return FALSE; } diff --git a/libfreerdp/core/gateway/rpc_client.c b/libfreerdp/core/gateway/rpc_client.c index bb807e250..27f3614f1 100644 --- a/libfreerdp/core/gateway/rpc_client.c +++ b/libfreerdp/core/gateway/rpc_client.c @@ -432,8 +432,7 @@ static int rpc_client_recv_fragment(rdpRpc* rpc, wStream* fragment) tsg_set_state(tsg, TSG_STATE_TUNNEL_CLOSE_PENDING); EventArgsInit(&e, "freerdp"); e.code = 0; - PubSub_OnTerminate(context->rdp->pubSub, context, &e); - rc = 0; + rc = PubSub_OnTerminate(context->rdp->pubSub, context, &e) >= 0 ? 0 : -1; goto success; } diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c index 98213985f..9f8591dd4 100644 --- a/libfreerdp/core/rdp.c +++ b/libfreerdp/core/rdp.c @@ -641,8 +641,7 @@ BOOL rdp_read_header(rdpRdp* rdp, wStream* s, UINT16* length, UINT16* channelId) utils_abort_connect(rdp); EventArgsInit(&e, "freerdp"); e.code = 0; - PubSub_OnTerminate(rdp->pubSub, context, &e); - return TRUE; + return PubSub_OnTerminate(rdp->pubSub, context, &e) >= 0; } if (!Stream_CheckAndLogRequiredLengthWLog(rdp->log, s, 5)) diff --git a/winpr/libwinpr/utils/collections/PubSub.c b/winpr/libwinpr/utils/collections/PubSub.c index d96b0b21a..8934a166c 100644 --- a/winpr/libwinpr/utils/collections/PubSub.c +++ b/winpr/libwinpr/utils/collections/PubSub.c @@ -23,6 +23,9 @@ #include +#include "../log.h" +#define TAG WINPR_TAG("pubsub") + /** * Events (C# Programming Guide) * http://msdn.microsoft.com/en-us/library/awbftdfh.aspx @@ -196,25 +199,23 @@ int PubSub_Unsubscribe(wPubSub* pubSub, const char* EventName, ...) int PubSub_OnEvent(wPubSub* pubSub, const char* EventName, void* context, const wEventArgs* e) { - wEventType* event = nullptr; - int status = -1; + WINPR_ASSERT(pubSub); + WINPR_ASSERT(EventName); + WINPR_ASSERT(e); if (!pubSub) + { + WLog_ERR(TAG, "pubSub(%s)=nullptr!", EventName); return -1; - WINPR_ASSERT(e); + } if (pubSub->synchronized) PubSub_Lock(pubSub); - event = PubSub_FindEventType(pubSub, EventName); - - if (pubSub->synchronized) - PubSub_Unlock(pubSub); - + int status = 0; + wEventType* event = PubSub_FindEventType(pubSub, EventName); if (event) { - status = 0; - for (size_t index = 0; index < event->EventHandlerCount; index++) { if (event->EventHandlers[index]) @@ -224,6 +225,8 @@ int PubSub_OnEvent(wPubSub* pubSub, const char* EventName, void* context, const } } } + if (pubSub->synchronized) + PubSub_Unlock(pubSub); return status; } diff --git a/winpr/libwinpr/utils/test/TestPubSub.c b/winpr/libwinpr/utils/test/TestPubSub.c index 39ef0e01f..2ff327a26 100644 --- a/winpr/libwinpr/utils/test/TestPubSub.c +++ b/winpr/libwinpr/utils/test/TestPubSub.c @@ -55,7 +55,8 @@ int TestPubSub(int argc, char* argv[]) e.x = 64; e.y = 128; - PubSub_OnMouseMotion(node, nullptr, &e); + if (PubSub_OnMouseMotion(node, nullptr, &e) < 0) + return -1; } { @@ -66,7 +67,8 @@ int TestPubSub(int argc, char* argv[]) e.flags = 7; e.button = 1; - PubSub_OnMouseButton(node, nullptr, &e); + if (PubSub_OnMouseButton(node, nullptr, &e) < 0) + return -1; } PubSub_Free(node); From ce3f205cd64d9f4fab1e83bbbd4717d0a4d1f7da Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 12:30:26 +0100 Subject: [PATCH 2/8] [core,gateway] fix missing return checks --- libfreerdp/core/gateway/rdg.c | 3 ++- libfreerdp/core/gateway/tsg.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libfreerdp/core/gateway/rdg.c b/libfreerdp/core/gateway/rdg.c index bdab4035b..2f827c799 100644 --- a/libfreerdp/core/gateway/rdg.c +++ b/libfreerdp/core/gateway/rdg.c @@ -2233,7 +2233,8 @@ rdpRdg* rdg_new(rdpContext* context) if (rdg->context->settings->GatewayAccessToken) rdg->extAuth = HTTP_EXTENDED_AUTH_PAA; - UuidCreate(&rdg->guid); + if (UuidCreate(&rdg->guid) != RPC_S_OK) + goto rdg_alloc_error; rdg->tlsOut = freerdp_tls_new(rdg->context); diff --git a/libfreerdp/core/gateway/tsg.c b/libfreerdp/core/gateway/tsg.c index 15012c90b..93d68e23a 100644 --- a/libfreerdp/core/gateway/tsg.c +++ b/libfreerdp/core/gateway/tsg.c @@ -1037,6 +1037,9 @@ static BOOL tsg_packet_quarenc_response_to_string(char** buffer, size_t* length, if (!tsg_print(buffer, length, " ")) return FALSE; + if (UuidToStringA(&caps->nonce, &uuid) != RPC_S_OK) + return FALSE; + if (caps->certChainLen > 0) { if (caps->certChainLen > INT_MAX) @@ -1047,7 +1050,6 @@ static BOOL tsg_packet_quarenc_response_to_string(char** buffer, size_t* length, } tsg_packet_versioncaps_to_string(&ptbuffer, &size, &caps->versionCaps); - UuidToStringA(&caps->nonce, &uuid); if (strdata || (caps->certChainLen == 0)) rc = tsg_print(buffer, length, From 1220c7af1cdc19d376e1560ea4aa809e294d6f07 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 12:30:40 +0100 Subject: [PATCH 3/8] [channels,video] fix missing return checks --- channels/video/client/video_main.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/channels/video/client/video_main.c b/channels/video/client/video_main.c index 0dc046f2b..c6081c21f 100644 --- a/channels/video/client/video_main.c +++ b/channels/video/client/video_main.c @@ -690,7 +690,7 @@ static void video_timer(VideoClientContext* video, UINT64 now) EnterCriticalSection(&priv->framesLock); do { - VideoFrame* peekFrame = (VideoFrame*)Queue_Peek(priv->frames); + const VideoFrame* peekFrame = (VideoFrame*)Queue_Peek(priv->frames); if (!peekFrame) break; @@ -703,8 +703,7 @@ static void video_timer(VideoClientContext* video, UINT64 now) priv->droppedFrames++; VideoFrame_free(&frame); } - frame = peekFrame; - Queue_Dequeue(priv->frames); + frame = Queue_Dequeue(priv->frames); } while (1); LeaveCriticalSection(&priv->framesLock); From 1fb02134e6db541a828b54fc076400f19bc923fe Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 12:40:07 +0100 Subject: [PATCH 4/8] [channels,drive] fix missing return checks --- channels/drive/client/drive_main.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c index 94b8b7f44..20a4472e4 100644 --- a/channels/drive/client/drive_main.c +++ b/channels/drive/client/drive_main.c @@ -256,7 +256,7 @@ static UINT drive_process_irp_close(DRIVE_DEVICE* drive, IRP* irp) irp->IoStatus = STATUS_UNSUCCESSFUL; else { - ListDictionary_Take(drive->files, key); + ListDictionary_Remove(drive->files, key); if (drive_file_free(file)) irp->IoStatus = STATUS_SUCCESS; @@ -470,8 +470,18 @@ static UINT drive_process_irp_query_volume_information(DRIVE_DEVICE* drive, IRP* return ERROR_INVALID_DATA; Stream_Read_UINT32(irp->input, FsInformationClass); - GetDiskFreeSpaceW(drive->path, &lpSectorsPerCluster, &lpBytesPerSector, &lpNumberOfFreeClusters, - &lpTotalNumberOfClusters); + if (!GetDiskFreeSpaceW(drive->path, &lpSectorsPerCluster, &lpBytesPerSector, + &lpNumberOfFreeClusters, &lpTotalNumberOfClusters)) + { + const UINT32 err = GetLastError(); + const HRESULT herr = HRESULT_FROM_WIN32(err); + WLog_WARN(TAG, "GetDiskFreeSpaceW failed: %s [%" PRId32 "], win32=%s [%" PRIu32 "]", + NtStatus2Tag(herr), herr, Win32ErrorCode2Tag(err & 0xFFFF), err); + lpSectorsPerCluster = 0; + lpBytesPerSector = 0; + lpNumberOfFreeClusters = 0; + lpTotalNumberOfClusters = 0; + } switch (FsInformationClass) { @@ -493,7 +503,18 @@ static UINT drive_process_irp_query_volume_information(DRIVE_DEVICE* drive, IRP* return CHANNEL_RC_NO_MEMORY; } - GetFileAttributesExW(drive->path, GetFileExInfoStandard, &wfad); + if (!GetFileAttributesExW(drive->path, GetFileExInfoStandard, &wfad)) + { + const UINT32 err = GetLastError(); + const HRESULT herr = HRESULT_FROM_WIN32(err); + WLog_WARN(TAG, + "GetFileAttributesExW failed: %s [%" PRId32 "], win32=%s [%" PRIu32 "]", + NtStatus2Tag(herr), herr, Win32ErrorCode2Tag(err & 0xFFFF), err); + + const WIN32_FILE_ATTRIBUTE_DATA empty = WINPR_C_ARRAY_INIT; + wfad = empty; + } + Stream_Write_UINT32(output, wfad.ftCreationTime.dwLowDateTime); /* VolumeCreationTime */ Stream_Write_UINT32(output, wfad.ftCreationTime.dwHighDateTime); /* VolumeCreationTime */ From 2b8ecce7a957a1f12b259ca05fdbb85cbcc9a899 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 12:45:17 +0100 Subject: [PATCH 5/8] [channels,rdpecam] fix return checks --- channels/rdpecam/client/camera_device_enum_main.c | 7 ++++++- channels/rdpecam/client/v4l/camera_v4l.c | 11 +++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/channels/rdpecam/client/camera_device_enum_main.c b/channels/rdpecam/client/camera_device_enum_main.c index 111863f75..85f67b5f8 100644 --- a/channels/rdpecam/client/camera_device_enum_main.c +++ b/channels/rdpecam/client/camera_device_enum_main.c @@ -352,7 +352,12 @@ static UINT ecam_plugin_initialize(IWTSPlugin* pPlugin, IWTSVirtualChannelManage return CHANNEL_RC_NO_MEMORY; } - HashTable_SetupForStringData(ecam->devices, FALSE); + if (!HashTable_SetupForStringData(ecam->devices, FALSE)) + { + HashTable_Free(ecam->devices); + ecam->devices = nullptr; + return ERROR_INTERNAL_ERROR; + } wObject* obj = HashTable_ValueObject(ecam->devices); WINPR_ASSERT(obj); diff --git a/channels/rdpecam/client/v4l/camera_v4l.c b/channels/rdpecam/client/v4l/camera_v4l.c index 034516b90..5494c233b 100644 --- a/channels/rdpecam/client/v4l/camera_v4l.c +++ b/channels/rdpecam/client/v4l/camera_v4l.c @@ -791,7 +791,7 @@ static CAM_ERROR_CODE cam_v4l_free(ICamHal* ihal) FREERDP_ENTRY_POINT(UINT VCAPITYPE v4l_freerdp_rdpecam_client_subsystem_entry( PFREERDP_CAMERA_HAL_ENTRY_POINTS pEntryPoints)) { - UINT ret = CHANNEL_RC_OK; + UINT ret = ERROR_INTERNAL_ERROR; WINPR_ASSERT(pEntryPoints); CamV4lHal* hal = (CamV4lHal*)calloc(1, sizeof(CamV4lHal)); @@ -809,18 +809,17 @@ FREERDP_ENTRY_POINT(UINT VCAPITYPE v4l_freerdp_rdpecam_client_subsystem_entry( hal->streams = HashTable_New(FALSE); if (!hal->streams) - { - ret = CHANNEL_RC_NO_MEMORY; goto error; - } - HashTable_SetupForStringData(hal->streams, FALSE); + if (!HashTable_SetupForStringData(hal->streams, FALSE)) + goto error; wObject* obj = HashTable_ValueObject(hal->streams); WINPR_ASSERT(obj); obj->fnObjectFree = cam_v4l_stream_free; - if ((ret = pEntryPoints->pRegisterCameraHal(pEntryPoints->plugin, &hal->iHal))) + ret = pEntryPoints->pRegisterCameraHal(pEntryPoints->plugin, &hal->iHal); + if (ret != CHANNEL_RC_OK) { WLog_ERR(TAG, "RegisterCameraHal failed with error %" PRIu32 "", ret); goto error; From 125b1ce99edd24636670eef58784eb7209d1af1b Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 13:18:36 +0100 Subject: [PATCH 6/8] [utils,ringbuffer] fix debug log messages Renaming some variables also in debug messages --- libfreerdp/utils/ringbuffer.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/libfreerdp/utils/ringbuffer.c b/libfreerdp/utils/ringbuffer.c index 76c9ff82e..f202b56bb 100644 --- a/libfreerdp/utils/ringbuffer.c +++ b/libfreerdp/utils/ringbuffer.c @@ -49,7 +49,7 @@ BOOL ringbuffer_init(RingBuffer* ringbuffer, size_t initialSize) ringbuffer->readPtr = ringbuffer->writePtr = 0; ringbuffer->initialSize = ringbuffer->size = ringbuffer->freeSize = initialSize; - DEBUG_RINGBUFFER("ringbuffer_init(%p)", (void*)rb); + DEBUG_RINGBUFFER("ringbuffer_init(%p)", (void*)ringbuffer); return TRUE; } @@ -79,7 +79,8 @@ static BOOL ringbuffer_realloc(RingBuffer* ringbuffer, size_t targetSize) { WINPR_ASSERT(ringbuffer); BYTE* newData = nullptr; - DEBUG_RINGBUFFER("ringbuffer_realloc(%p): targetSize: %" PRIdz "", (void*)rb, targetSize); + DEBUG_RINGBUFFER("ringbuffer_realloc(%p): targetSize: %" PRIdz "", (void*)ringbuffer, + targetSize); if (ringbuffer->writePtr == ringbuffer->readPtr) { @@ -172,7 +173,7 @@ BOOL ringbuffer_write(RingBuffer* ringbuffer, const BYTE* ptr, size_t sz) size_t remaining = 0; WINPR_ASSERT(ringbuffer); - DEBUG_RINGBUFFER("ringbuffer_write(%p): sz: %" PRIdz "", (void*)rb, sz); + DEBUG_RINGBUFFER("ringbuffer_write(%p): sz: %" PRIdz "", (void*)ringbuffer, sz); if ((ringbuffer->freeSize <= sz) && !ringbuffer_realloc(ringbuffer, ringbuffer->size + sz)) return FALSE; @@ -206,7 +207,7 @@ BOOL ringbuffer_write(RingBuffer* ringbuffer, const BYTE* ptr, size_t sz) BYTE* ringbuffer_ensure_linear_write(RingBuffer* ringbuffer, size_t sz) { - DEBUG_RINGBUFFER("ringbuffer_ensure_linear_write(%p): sz: %" PRIdz "", (void*)rb, sz); + DEBUG_RINGBUFFER("ringbuffer_ensure_linear_write(%p): sz: %" PRIdz "", (void*)ringbuffer, sz); WINPR_ASSERT(ringbuffer); if (ringbuffer->freeSize < sz) @@ -239,7 +240,7 @@ BYTE* ringbuffer_ensure_linear_write(RingBuffer* ringbuffer, size_t sz) BOOL ringbuffer_commit_written_bytes(RingBuffer* ringbuffer, size_t sz) { - DEBUG_RINGBUFFER("ringbuffer_commit_written_bytes(%p): sz: %" PRIdz "", (void*)rb, sz); + DEBUG_RINGBUFFER("ringbuffer_commit_written_bytes(%p): sz: %" PRIdz "", (void*)ringbuffer, sz); WINPR_ASSERT(ringbuffer); if (sz < 1) @@ -259,7 +260,7 @@ int ringbuffer_peek(const RingBuffer* ringbuffer, DataChunk chunks[2], size_t sz size_t toRead = 0; int chunkIndex = 0; int status = 0; - DEBUG_RINGBUFFER("ringbuffer_peek(%p): sz: %" PRIdz "", (const void*)rb, sz); + DEBUG_RINGBUFFER("ringbuffer_peek(%p): sz: %" PRIdz "", (const void*)ringbuffer, sz); WINPR_ASSERT(ringbuffer); if (sz < 1) From a9e0abf2eac8c2e370fa155bf1abb9d044c0ca8a Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 13:58:09 +0100 Subject: [PATCH 7/8] [core,orders] improve input validation check length before subtracting. Might underflow and be cought by the next check, but lets be strict. --- libfreerdp/core/orders.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index 327baae6a..21eb79d0e 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -2354,6 +2354,8 @@ static CACHE_BITMAP_ORDER* update_read_cache_bitmap_order(rdpUpdate* update, wSt goto fail; Stream_Read(s, bitmapComprHdr, 8); /* bitmapComprHdr (8 bytes) */ + if (cache_bitmap->bitmapLength < 8) + goto fail; cache_bitmap->bitmapLength -= 8; } } From 81526b19a2933e93d37d2007f4fe24e9df699123 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 3 Mar 2026 14:07:49 +0100 Subject: [PATCH 8/8] [string,format] fix format strings for debug messages --- channels/rdpei/client/rdpei_main.c | 2 +- libfreerdp/locale/keyboard_xkbfile.c | 2 +- winpr/libwinpr/sspi/NTLM/ntlm.c | 4 ++-- winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c | 3 ++- winpr/libwinpr/synch/event.c | 5 +++-- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/channels/rdpei/client/rdpei_main.c b/channels/rdpei/client/rdpei_main.c index f727b1eed..8213c63bb 100644 --- a/channels/rdpei/client/rdpei_main.c +++ b/channels/rdpei/client/rdpei_main.c @@ -239,7 +239,7 @@ static UINT rdpei_send_pdu(GENERIC_CHANNEL_CALLBACK* callback, wStream* s, UINT1 Stream_Buffer(s), nullptr); #ifdef WITH_DEBUG_RDPEI WLog_Print(rdpei->base.log, WLOG_DEBUG, - "rdpei_send_pdu: eventId: %" PRIu16 " (%s) length: %" PRIu32 " status: %" PRIu32 "", + "rdpei_send_pdu: eventId: %" PRIu16 " (%s) length: %" PRIuz " status: %" PRIu32 "", eventId, rdpei_eventid_string(eventId), pduLength, status); #endif return status; diff --git a/libfreerdp/locale/keyboard_xkbfile.c b/libfreerdp/locale/keyboard_xkbfile.c index 453208996..73cc42f10 100644 --- a/libfreerdp/locale/keyboard_xkbfile.c +++ b/libfreerdp/locale/keyboard_xkbfile.c @@ -498,7 +498,7 @@ int freerdp_keyboard_load_map_from_xkbfile(void* display, DWORD* x11_keycode_to_ if (!found) { - DEBUG_KBD("%4s: keycode: 0x%02X -> no RDP scancode found", xkb_keyname, i); + DEBUG_KBD("%4s: keycode: 0x%02" PRIxz " -> no RDP scancode found", xkb_keyname, i); } else status = 0; diff --git a/winpr/libwinpr/sspi/NTLM/ntlm.c b/winpr/libwinpr/sspi/NTLM/ntlm.c index 9f73343f0..2aa9c95a2 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm.c @@ -1170,7 +1170,7 @@ hmac_fail: } #ifdef WITH_DEBUG_NTLM - WLog_DBG(TAG, "Data Buffer (length = %" PRIuz ")", length); + WLog_DBG(TAG, "Data Buffer (length = %" PRIu32 ")", length); winpr_HexDump(TAG, WLOG_DEBUG, data, length); WLog_DBG(TAG, "Encrypted Data Buffer (length = %" PRIu32 ")", data_buffer->cbBuffer); winpr_HexDump(TAG, WLOG_DEBUG, data_buffer->pvBuffer, data_buffer->cbBuffer); @@ -1279,7 +1279,7 @@ hmac_fail: } #ifdef WITH_DEBUG_NTLM - WLog_DBG(TAG, "Encrypted Data Buffer (length = %" PRIuz ")", length); + WLog_DBG(TAG, "Encrypted Data Buffer (length = %" PRIu32 ")", length); winpr_HexDump(TAG, WLOG_DEBUG, data, length); WLog_DBG(TAG, "Data Buffer (length = %" PRIu32 ")", data_buffer->cbBuffer); winpr_HexDump(TAG, WLOG_DEBUG, data_buffer->pvBuffer, data_buffer->cbBuffer); diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c index 8c4609494..83ebc19fb 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c @@ -175,7 +175,8 @@ void ntlm_print_av_pair_list(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList) size_t cbLen = 0; ntlm_av_pair_get_len(pAvPair, cbAvPair, &cbLen); - WLog_VRB(TAG, "\t%s AvId: %" PRIu16 " AvLen: %" PRIu16 "", get_av_pair_string(pair), pair); + WLog_VRB(TAG, "\t%s AvId: %" PRIu16 " AvLen: %" PRIuz "", get_av_pair_string(pair), pair, + cbLen); winpr_HexDump(TAG, WLOG_TRACE, ntlm_av_pair_get_value_pointer(pAvPair), cbLen); pAvPair = ntlm_av_pair_next(pAvPair, &cbAvPair); diff --git a/winpr/libwinpr/synch/event.c b/winpr/libwinpr/synch/event.c index 62556bf95..a5f0422c3 100644 --- a/winpr/libwinpr/synch/event.c +++ b/winpr/libwinpr/synch/event.c @@ -57,7 +57,7 @@ static void dump_event(WINPR_EVENT* event, size_t index) char** msg = nullptr; size_t used = 0; - WLog_DBG(TAG, "Event handle created still not closed! [%" PRIuz ", %p]", index, event); + WLog_DBG(TAG, "Event handle created still not closed! [%" PRIuz ", %p]", index, (void*)event); msg = winpr_backtrace_symbols(event->create_stack, &used); for (size_t i = 2; i < used; i++) @@ -545,7 +545,8 @@ void DumpEventHandles_(const char* fkt, const char* file, size_t line) if (flags >= 0) count++; } - WLog_INFO(TAG, "------- limits [%d/%d] open files %" PRIuz, r.rlim_cur, r.rlim_max, count); + WLog_INFO(TAG, "------- limits [%lu/%lu] open files %" PRIuz, (unsigned long)r.rlim_cur, + (unsigned long)r.rlim_max, count); } WLog_DBG(TAG, "--------- Start dump [%s %s:%" PRIuz "]", fkt, file, line); if (global_event_list)