From cc2321d359fbb72fa5cd1d8c0c0e3ccee6cbf8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Thu, 11 Dec 2014 17:08:22 -0500 Subject: [PATCH] libfreerdp-core: fix leak and use after free in tsg ListDictionary usage --- libfreerdp/core/gateway/http.c | 6 ++--- libfreerdp/core/gateway/ncacn_http.c | 22 ++++++++++++------- libfreerdp/core/gateway/rts.c | 22 ++++++++++--------- libfreerdp/core/gateway/rts.h | 11 ---------- .../utils/collections/ListDictionary.c | 5 +++++ 5 files changed, 33 insertions(+), 33 deletions(-) diff --git a/libfreerdp/core/gateway/http.c b/libfreerdp/core/gateway/http.c index 7fb91d1e4..9e3aadf9c 100644 --- a/libfreerdp/core/gateway/http.c +++ b/libfreerdp/core/gateway/http.c @@ -402,8 +402,6 @@ BOOL http_response_parse_header_field(HttpResponse* http_response, char* name, c } status = ListDictionary_Add(http_response->Authenticates, authScheme, authValue); - - free(authScheme); } return status; @@ -571,7 +569,7 @@ HttpResponse* http_response_recv(rdpTls* tls) if (count) { - http_response->lines = (char**)calloc(http_response->count, sizeof(char*)); + http_response->lines = (char**) calloc(http_response->count, sizeof(char*)); if (!http_response->lines) goto out_error; @@ -598,7 +596,7 @@ HttpResponse* http_response_recv(rdpTls* tls) if (http_response->bodyLen > 0) { - http_response->BodyContent = (BYTE*)malloc(http_response->bodyLen); + http_response->BodyContent = (BYTE*) malloc(http_response->bodyLen); if (!http_response->BodyContent) goto out_error; diff --git a/libfreerdp/core/gateway/ncacn_http.c b/libfreerdp/core/gateway/ncacn_http.c index d06e9d24e..d9bdf48d8 100644 --- a/libfreerdp/core/gateway/ncacn_http.c +++ b/libfreerdp/core/gateway/ncacn_http.c @@ -92,18 +92,21 @@ int rpc_ncacn_http_send_in_channel_request(rdpRpc* rpc) int rpc_ncacn_http_recv_in_channel_response(rdpRpc* rpc) { + char* token64; int ntlm_token_length = 0; BYTE* ntlm_token_data = NULL; HttpResponse* http_response; rdpNtlm* ntlm = rpc->NtlmHttpIn->ntlm; http_response = http_response_recv(rpc->TlsIn); + if (!http_response) return -1; if (ListDictionary_Contains(http_response->Authenticates, "NTLM")) { - char *token64 = ListDictionary_GetItemValue(http_response->Authenticates, "NTLM"); + token64 = ListDictionary_GetItemValue(http_response->Authenticates, "NTLM"); + if (!token64) goto out; @@ -135,7 +138,7 @@ int rpc_ncacn_http_ntlm_init(rdpRpc* rpc, TSG_CHANNEL channel) if (instance->GatewayAuthenticate) { BOOL proceed = instance->GatewayAuthenticate(instance, &settings->GatewayUsername, - &settings->GatewayPassword, &settings->GatewayDomain); + &settings->GatewayPassword, &settings->GatewayDomain); if (!proceed) { @@ -170,8 +173,8 @@ int rpc_ncacn_http_ntlm_init(rdpRpc* rpc, TSG_CHANNEL channel) BOOL rpc_ntlm_http_in_connect(rdpRpc* rpc) { - rdpNtlm* ntlm = rpc->NtlmHttpIn->ntlm; BOOL success = FALSE; + rdpNtlm* ntlm = rpc->NtlmHttpIn->ntlm; if (rpc_ncacn_http_ntlm_init(rpc, TSG_CHANNEL_IN) == 1) { @@ -221,6 +224,7 @@ int rpc_ncacn_http_send_out_channel_request(rdpRpc* rpc) int rpc_ncacn_http_recv_out_channel_response(rdpRpc* rpc) { + char* token64; int ntlm_token_length = 0; BYTE* ntlm_token_data = NULL; HttpResponse* http_response; @@ -228,11 +232,12 @@ int rpc_ncacn_http_recv_out_channel_response(rdpRpc* rpc) http_response = http_response_recv(rpc->TlsOut); - ntlm_token_data = NULL; + if (!http_response) + return -1; - if (http_response && ListDictionary_Contains(http_response->Authenticates, "NTLM")) + if (ListDictionary_Contains(http_response->Authenticates, "NTLM")) { - char* token64 = ListDictionary_GetItemValue(http_response->Authenticates, "NTLM"); + token64 = ListDictionary_GetItemValue(http_response->Authenticates, "NTLM"); crypto_base64_decode(token64, strlen(token64), &ntlm_token_data, &ntlm_token_length); } @@ -240,6 +245,7 @@ int rpc_ncacn_http_recv_out_channel_response(rdpRpc* rpc) ntlm->inputBuffer[0].cbBuffer = ntlm_token_length; http_response_free(http_response); + return 0; } @@ -287,11 +293,11 @@ void rpc_ntlm_http_init_channel(rdpRpc* rpc, rdpNtlmHttp* ntlm_http, TSG_CHANNEL if (channel == TSG_CHANNEL_IN) { - http_context_set_pragma(ntlm_http->context, "ResourceTypeUuid=44e265dd-7daf-42cd-8560-3cdb6e7a2729"); + http_context_set_pragma(ntlm_http->context, "ResourceTypeUuid=44e265dd-7daf-42cd-8560-3cdb6e7a2729"); } else if (channel == TSG_CHANNEL_OUT) { - http_context_set_pragma(ntlm_http->context, "ResourceTypeUuid=44e265dd-7daf-42cd-8560-3cdb6e7a2729, " + http_context_set_pragma(ntlm_http->context, "ResourceTypeUuid=44e265dd-7daf-42cd-8560-3cdb6e7a2729, " "SessionId=fbd9c34f-397d-471d-a109-1b08cc554624"); } } diff --git a/libfreerdp/core/gateway/rts.c b/libfreerdp/core/gateway/rts.c index 4652d6a97..b9e2ead5e 100644 --- a/libfreerdp/core/gateway/rts.c +++ b/libfreerdp/core/gateway/rts.c @@ -65,6 +65,8 @@ BOOL rts_connect(rdpRpc* rpc) RPC_PDU* pdu; rpcconn_rts_hdr_t* rts; HttpResponse* http_response; + freerdp* instance = (freerdp*) rpc->settings->instance; + rdpContext* context = instance->context; /** * Connection Opening @@ -90,7 +92,7 @@ BOOL rts_connect(rdpRpc* rpc) */ rpc->VirtualConnection->State = VIRTUAL_CONNECTION_STATE_INITIAL; - DEBUG_RTS("VIRTUAL_CONNECTION_STATE_INITIAL"); + WLog_DBG(TAG, "VIRTUAL_CONNECTION_STATE_INITIAL"); rpc->client->SynchronousSend = TRUE; rpc->client->SynchronousReceive = TRUE; @@ -120,7 +122,7 @@ BOOL rts_connect(rdpRpc* rpc) } rpc->VirtualConnection->State = VIRTUAL_CONNECTION_STATE_OUT_CHANNEL_WAIT; - DEBUG_RTS("VIRTUAL_CONNECTION_STATE_OUT_CHANNEL_WAIT"); + WLog_DBG(TAG, "VIRTUAL_CONNECTION_STATE_OUT_CHANNEL_WAIT"); /** * Receive OUT Channel Response @@ -170,9 +172,9 @@ BOOL rts_connect(rdpRpc* rpc) connectErrorCode = AUTHENTICATIONERROR; } - if (!freerdp_get_last_error(((freerdp*)(rpc->settings->instance))->context)) + if (!freerdp_get_last_error(context)) { - freerdp_set_last_error(((freerdp*)(rpc->settings->instance))->context, FREERDP_ERROR_AUTHENTICATION_FAILED); + freerdp_set_last_error(context, FREERDP_ERROR_AUTHENTICATION_FAILED); } } @@ -191,7 +193,7 @@ BOOL rts_connect(rdpRpc* rpc) http_response_free(http_response); rpc->VirtualConnection->State = VIRTUAL_CONNECTION_STATE_WAIT_A3W; - DEBUG_RTS("VIRTUAL_CONNECTION_STATE_WAIT_A3W"); + WLog_DBG(TAG, "VIRTUAL_CONNECTION_STATE_WAIT_A3W"); /** * Receive CONN_A3 RTS PDU @@ -229,7 +231,7 @@ BOOL rts_connect(rdpRpc* rpc) rpc_client_receive_pool_return(rpc, pdu); rpc->VirtualConnection->State = VIRTUAL_CONNECTION_STATE_WAIT_C2; - DEBUG_RTS("VIRTUAL_CONNECTION_STATE_WAIT_C2"); + WLog_DBG(TAG, "VIRTUAL_CONNECTION_STATE_WAIT_C2"); /** * Receive CONN_C2 RTS PDU @@ -269,7 +271,7 @@ BOOL rts_connect(rdpRpc* rpc) rpc_client_receive_pool_return(rpc, pdu); rpc->VirtualConnection->State = VIRTUAL_CONNECTION_STATE_OPENED; - DEBUG_RTS("VIRTUAL_CONNECTION_STATE_OPENED"); + WLog_DBG(TAG, "VIRTUAL_CONNECTION_STATE_OPENED"); rpc->client->SynchronousSend = TRUE; rpc->client->SynchronousReceive = TRUE; @@ -720,7 +722,7 @@ int rts_recv_CONN_A3_pdu(rdpRpc* rpc, BYTE* buffer, UINT32 length) rts_connection_timeout_command_read(rpc, &buffer[24], length - 24, &ConnectionTimeout); - DEBUG_RTS("ConnectionTimeout: %d", ConnectionTimeout); + WLog_DBG(TAG, "ConnectionTimeout: %d", ConnectionTimeout); rpc->VirtualConnection->DefaultInChannel->PingOriginator.ConnectionTimeout = ConnectionTimeout; @@ -787,8 +789,8 @@ int rts_recv_CONN_C2_pdu(rdpRpc* rpc, BYTE* buffer, UINT32 length) offset += rts_receive_window_size_command_read(rpc, &buffer[offset], length - offset, &ReceiveWindowSize) + 4; offset += rts_connection_timeout_command_read(rpc, &buffer[offset], length - offset, &ConnectionTimeout) + 4; - DEBUG_RTS("ConnectionTimeout: %d", ConnectionTimeout); - DEBUG_RTS("ReceiveWindowSize: %d", ReceiveWindowSize); + WLog_DBG(TAG, "ConnectionTimeout: %d", ConnectionTimeout); + WLog_DBG(TAG, "ReceiveWindowSize: %d", ReceiveWindowSize); /* TODO: verify if this is the correct protocol variable */ rpc->VirtualConnection->DefaultInChannel->PingOriginator.ConnectionTimeout = ConnectionTimeout; diff --git a/libfreerdp/core/gateway/rts.h b/libfreerdp/core/gateway/rts.h index a0fd620a9..f1c60c57a 100644 --- a/libfreerdp/core/gateway/rts.h +++ b/libfreerdp/core/gateway/rts.h @@ -143,15 +143,4 @@ int rts_recv_out_of_sequence_pdu(rdpRpc* rpc, BYTE* buffer, UINT32 length); #include "rts_signature.h" -#ifdef WITH_DEBUG_TSG -#define WITH_DEBUG_RTS -#endif - -#define RTS_TAG FREERDP_TAG("core.gateway.rts") -#ifdef WITH_DEBUG_RTS -#define DEBUG_RTS(fmt, ...) WLog_DBG(RTS_TAG, fmt, ## __VA_ARGS__) -#else -#define DEBUG_RTS(fmt, ...) do { } while (0) -#endif - #endif /* FREERDP_CORE_RTS_H */ diff --git a/winpr/libwinpr/utils/collections/ListDictionary.c b/winpr/libwinpr/utils/collections/ListDictionary.c index c1733a45b..7f922dfe6 100644 --- a/winpr/libwinpr/utils/collections/ListDictionary.c +++ b/winpr/libwinpr/utils/collections/ListDictionary.c @@ -229,8 +229,13 @@ void ListDictionary_Clear(wListDictionary* listDictionary) while (item) { nextItem = item->next; + + if (listDictionary->objectKey.fnObjectFree) + listDictionary->objectKey.fnObjectFree(item->key); + if (listDictionary->objectValue.fnObjectFree) listDictionary->objectValue.fnObjectFree(item->value); + free(item); item = nextItem; }