From 3bc47a2bf83a75b072de5414679b81bf3eadc373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Thu, 31 Oct 2013 21:12:06 -0400 Subject: [PATCH] libfreerdp-core: fix leaks and potential use after free --- client/X11/xf_client.c | 58 ++++++++++++++++++----------------- client/common/client.c | 2 +- client/common/compatibility.c | 5 +++ libfreerdp/core/freerdp.c | 3 ++ libfreerdp/core/gateway/tsg.c | 14 ++++++--- 5 files changed, 49 insertions(+), 33 deletions(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 8a29b2d5c..c586dbb58 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -750,13 +750,41 @@ BOOL xf_pre_connect(freerdp* instance) rdpSettings* settings; xfContext* xfc = (xfContext*) instance->context; - xfc->mutex = CreateMutex(NULL, FALSE, NULL); xfc->settings = instance->settings; xfc->instance = instance; settings = instance->settings; channels = instance->context->channels; + xfc->UseXThreads = TRUE; + + if (xfc->UseXThreads) + { + if (!XInitThreads()) + { + fprintf(stderr, "warning: XInitThreads() failure\n"); + xfc->UseXThreads = FALSE; + } + } + + xfc->display = XOpenDisplay(NULL); + + if (!xfc->display) + { + fprintf(stderr, "xf_pre_connect: failed to open display: %s\n", XDisplayName(NULL)); + fprintf(stderr, "Please check that the $DISPLAY environment variable is properly set.\n"); + return FALSE; + } + + if (xfc->debug) + { + fprintf(stderr, "Enabling X11 debug mode.\n"); + XSynchronize(xfc->display, TRUE); + _def_error_handler = XSetErrorHandler(_xf_error_handler); + } + + xfc->mutex = CreateMutex(NULL, FALSE, NULL); + PubSub_SubscribeChannelConnected(instance->context->pubSub, (pChannelConnectedEventHandler) xf_OnChannelConnectedEventHandler); @@ -785,33 +813,6 @@ BOOL xf_pre_connect(freerdp* instance) return TRUE; } - xfc->UseXThreads = TRUE; - - if (xfc->UseXThreads) - { - if (!XInitThreads()) - { - fprintf(stderr, "warning: XInitThreads() failure\n"); - xfc->UseXThreads = FALSE; - } - } - - xfc->display = XOpenDisplay(NULL); - - if (!xfc->display) - { - fprintf(stderr, "xf_pre_connect: failed to open display: %s\n", XDisplayName(NULL)); - fprintf(stderr, "Please check that the $DISPLAY environment variable is properly set.\n"); - return FALSE; - } - - if (xfc->debug) - { - fprintf(stderr, "Enabling X11 debug mode.\n"); - XSynchronize(xfc->display, TRUE); - _def_error_handler = XSetErrorHandler(_xf_error_handler); - } - xfc->_NET_WM_ICON = XInternAtom(xfc->display, "_NET_WM_ICON", False); xfc->_MOTIF_WM_HINTS = XInternAtom(xfc->display, "_MOTIF_WM_HINTS", False); xfc->_NET_CURRENT_DESKTOP = XInternAtom(xfc->display, "_NET_CURRENT_DESKTOP", False); @@ -1754,6 +1755,7 @@ static int xfreerdp_client_stop(rdpContext* context) xfContext* xfc = (xfContext*) context; assert(NULL != context); + if (context->settings->AsyncInput) { wMessageQueue* queue; diff --git a/client/common/client.c b/client/common/client.c index 926ea9889..f563f198d 100644 --- a/client/common/client.c +++ b/client/common/client.c @@ -70,12 +70,12 @@ rdpContext* freerdp_client_context_new(RDP_CLIENT_ENTRY_POINTS* pEntryPoints) void freerdp_client_context_free(rdpContext* context) { freerdp* instance = context->instance; + if (instance) { freerdp_context_free(instance); free(instance->pClientEntryPoints); freerdp_free(instance); - context->instance = NULL; } } diff --git a/client/common/compatibility.c b/client/common/compatibility.c index 5fdd9ea3a..279706722 100644 --- a/client/common/compatibility.c +++ b/client/common/compatibility.c @@ -321,10 +321,15 @@ int freerdp_detect_old_command_line_syntax(int argc, char** argv, int* count) ZeroMemory(settings, sizeof(rdpSettings)); CommandLineClearArgumentsA(old_args); + status = CommandLineParseArgumentsA(argc, (const char**) argv, old_args, flags, settings, freerdp_client_old_command_line_pre_filter, NULL); + if (status < 0) + { + free(settings); return status; + } arg = old_args; diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index c416c51b9..db8f4450c 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -428,6 +428,9 @@ int freerdp_context_new(freerdp* instance) */ void freerdp_context_free(freerdp* instance) { + if (!instance) + return; + if (!instance->context) return; diff --git a/libfreerdp/core/gateway/tsg.c b/libfreerdp/core/gateway/tsg.c index e10e164c8..896c6ce29 100644 --- a/libfreerdp/core/gateway/tsg.c +++ b/libfreerdp/core/gateway/tsg.c @@ -350,23 +350,29 @@ BOOL TsProxyCreateTunnelReadResponse(rdpTsg* tsg, RPC_PDU* pdu) offset += 4; Pointer = *((UINT32*) &buffer[offset]); offset += 4; - if(Pointer) { + + if (Pointer) + { offset += 4; // MaxCount offset += 8; // UnicodeString Offset, Length } - if(MsgBytes > TSG_MESSAGING_MAX_MESSAGE_LENGTH) { - fprintf(stderr, "Out of Spec Message Length %d"); + + if (MsgBytes > TSG_MESSAGING_MAX_MESSAGE_LENGTH) + { + fprintf(stderr, "Out of Spec Message Length %d", MsgBytes); return FALSE; } offset += MsgBytes; break; + case TSG_ASYNC_MESSAGE_REAUTH: rpc_offset_align(&offset, 8); offset += 8; // UINT64 TunnelContext, not to be confused with // the ContextHandle TunnelContext below. break; + default: - fprintf(stderr, "Unexpected Message Type: 0x%X\n", MessageSwitchValue); + fprintf(stderr, "Unexpected Message Type: 0x%X\n", (int) MessageSwitchValue); return FALSE; }