Merge pull request #1787 from hardening/oom_checks2

Misc fixes to check OOM

libfreerdp/core/connection.c

@@ -169,6 +169,7 @@
 
 BOOL rdp_client_connect(rdpRdp* rdp)
 {
+	BOOL ret;
 	rdpSettings* settings = rdp->settings;
 
 	if (rdp->settingsCopy)
@@ -208,6 +209,8 @@ BOOL rdp_client_connect(rdpRdp* rdp)
 
 		cookie_length = domain_length + 1 + user_length;
 		cookie = (char*) malloc(cookie_length + 1);
+		if (!cookie)
+			return FALSE;
 
 		CopyMemory(cookie, domain, domain_length);
 		CharUpperBuffA(cookie, domain_length);
@@ -218,14 +221,17 @@ BOOL rdp_client_connect(rdpRdp* rdp)
 
 		cookie[cookie_length] = '\0';
 
-		nego_set_cookie(rdp->nego, cookie);
+		ret = nego_set_cookie(rdp->nego, cookie);
 		free(cookie);
 	}
 	else
 	{
-		nego_set_cookie(rdp->nego, settings->Username);
+		ret = nego_set_cookie(rdp->nego, settings->Username);
 	}
 
+	if (!ret)
+		return FALSE;
+
 	nego_set_send_preconnection_pdu(rdp->nego, settings->SendPreconnectionPdu);
 	nego_set_preconnection_id(rdp->nego, settings->PreconnectionId);
 	nego_set_preconnection_blob(rdp->nego, settings->PreconnectionBlob);
@@ -247,7 +253,10 @@ BOOL rdp_client_connect(rdpRdp* rdp)
 	nego_set_cookie_max_length(rdp->nego, settings->CookieMaxLength);
 
 	if (settings->LoadBalanceInfo)
-		nego_set_routing_token(rdp->nego, settings->LoadBalanceInfo, settings->LoadBalanceInfoLength);
+	{
+		if (!nego_set_routing_token(rdp->nego, settings->LoadBalanceInfo, settings->LoadBalanceInfoLength))
+			return FALSE;
+	}
 
 	if (!nego_connect(rdp->nego))
 	{
@@ -525,7 +534,10 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s)
 	}
 
 	if (!rdp_read_security_header(s, &sec_flags))
+	{
+		fprintf(stderr, "%s: invalid security header\n", __FUNCTION__);
 		return FALSE;
+	}
 
 	if ((sec_flags & SEC_EXCHANGE_PKT) == 0)
 	{

libfreerdp/core/gateway/ntlm.c

@@ -137,6 +137,8 @@ BOOL ntlm_client_make_spn(rdpNtlm* ntlm, LPCTSTR ServiceClass, char* hostname)
 	if (!ServiceClass)
 	{
 		ntlm->ServicePrincipalName = (LPTSTR) _tcsdup(hostnameX);
+		if (!ntlm->ServicePrincipalName)
+			return FALSE;
 		return TRUE;
 	}
 
@@ -147,6 +149,8 @@ BOOL ntlm_client_make_spn(rdpNtlm* ntlm, LPCTSTR ServiceClass, char* hostname)
 		return FALSE;
 
 	ntlm->ServicePrincipalName = (LPTSTR) malloc(SpnLength * sizeof(TCHAR));
+	if (!ntlm->ServicePrincipalName)
+		return FALSE;
 
 	status = DsMakeSpn(ServiceClass, hostnameX, NULL, 0, NULL, &SpnLength, ntlm->ServicePrincipalName);
 
@@ -210,6 +214,8 @@ BOOL ntlm_authenticate(rdpNtlm* ntlm)
 	ntlm->outputBuffer[0].BufferType = SECBUFFER_TOKEN;
 	ntlm->outputBuffer[0].cbBuffer = ntlm->cbMaxToken;
 	ntlm->outputBuffer[0].pvBuffer = malloc(ntlm->outputBuffer[0].cbBuffer);
+	if (!ntlm->outputBuffer[0].pvBuffer)
+		return FALSE;
 
 	if (ntlm->haveInputBuffer)
 	{

libfreerdp/core/gateway/rpc.c

@@ -497,20 +497,27 @@ void rpc_client_virtual_connection_init(rdpRpc* rpc, RpcVirtualConnection* conne
 
 RpcVirtualConnection* rpc_client_virtual_connection_new(rdpRpc* rpc)
 {
-	RpcVirtualConnection* connection = (RpcVirtualConnection*) malloc(sizeof(RpcVirtualConnection));
+	RpcVirtualConnection* connection = (RpcVirtualConnection*) calloc(1, sizeof(RpcVirtualConnection));
+
+	if (!connection)
+		return NULL;
 
-	if (connection != NULL)
-	{
-		ZeroMemory(connection, sizeof(RpcVirtualConnection));
 	connection->State = VIRTUAL_CONNECTION_STATE_INITIAL;
-		connection->DefaultInChannel = (RpcInChannel*) malloc(sizeof(RpcInChannel));
+	connection->DefaultInChannel = (RpcInChannel *)calloc(1, sizeof(RpcInChannel));
-		connection->DefaultOutChannel = (RpcOutChannel*) malloc(sizeof(RpcOutChannel));
+	if (!connection->DefaultInChannel)
-		ZeroMemory(connection->DefaultInChannel, sizeof(RpcInChannel));
+		goto out_free;
-		ZeroMemory(connection->DefaultOutChannel, sizeof(RpcOutChannel));
+	connection->DefaultOutChannel = (RpcOutChannel*)calloc(1, sizeof(RpcOutChannel));
+	if (!connection->DefaultOutChannel)
+		goto out_default_in;
 	rpc_client_virtual_connection_init(rpc, connection);
-	}
 
 	return connection;
+
+out_default_in:
+	free(connection->DefaultInChannel);
+out_free:
+	free(connection);
+	return NULL;
 }
 
 void rpc_client_virtual_connection_free(RpcVirtualConnection* virtual_connection)
@@ -525,11 +532,9 @@ void rpc_client_virtual_connection_free(RpcVirtualConnection* virtual_connection
 
 rdpRpc* rpc_new(rdpTransport* transport)
 {
-	rdpRpc* rpc = (rdpRpc*) malloc(sizeof(rdpRpc));
+	rdpRpc* rpc = (rdpRpc*) calloc(1, sizeof(rdpRpc));
-
+	if (!rpc)
-	if (rpc != NULL)
+		return NULL;
-	{
-		ZeroMemory(rpc, sizeof(rdpRpc));
 
 	rpc->State = RPC_CLIENT_STATE_INITIAL;
 
@@ -538,9 +543,15 @@ rdpRpc* rpc_new(rdpTransport* transport)
 
 	rpc->SendSeqNum = 0;
 	rpc->ntlm = ntlm_new();
+	if (!rpc->ntlm)
+		goto out_free;
 
 	rpc->NtlmHttpIn = ntlm_http_new();
+	if (!rpc->NtlmHttpIn)
+		goto out_free_ntlm;
 	rpc->NtlmHttpOut = ntlm_http_new();
+	if (!rpc->NtlmHttpOut)
+		goto out_free_ntlm_http_in;
 
 	rpc_ntlm_http_init_channel(rpc, rpc->NtlmHttpIn, TSG_CHANNEL_IN);
 	rpc_ntlm_http_init_channel(rpc, rpc->NtlmHttpOut, TSG_CHANNEL_OUT);
@@ -572,7 +583,12 @@ rdpRpc* rpc_new(rdpTransport* transport)
 	rpc->CurrentKeepAliveTime = 0;
 
 	rpc->VirtualConnection = rpc_client_virtual_connection_new(rpc);
+	if (!rpc->VirtualConnection)
+		goto out_free_ntlm_http_out;
+
 	rpc->VirtualConnectionCookieTable = ArrayList_New(TRUE);
+	if (!rpc->VirtualConnectionCookieTable)
+		goto out_free_virtual_connection;
 
 	rpc->CallId = 2;
 
@@ -580,9 +596,20 @@ rdpRpc* rpc_new(rdpTransport* transport)
 
 	rpc->client->SynchronousSend = TRUE;
 	rpc->client->SynchronousReceive = TRUE;
-	}
 
 	return rpc;
+
+out_free_virtual_connection:
+	rpc_client_virtual_connection_free(rpc->VirtualConnection);
+out_free_ntlm_http_out:
+	ntlm_http_free(rpc->NtlmHttpOut);
+out_free_ntlm_http_in:
+	ntlm_http_free(rpc->NtlmHttpIn);
+out_free_ntlm:
+	ntlm_free(rpc->ntlm);
+out_free:
+	free(rpc);
+	return NULL;
 }
 
 void rpc_free(rdpRpc* rpc)

libfreerdp/core/nego.c

@@ -88,6 +88,7 @@ BOOL nego_connect(rdpNego* nego)
 		{
 			DEBUG_NEGO("No security protocol is enabled");
 			nego->state = NEGO_STATE_FAIL;
+			return FALSE;
 		}
 
 		if (!nego->NegotiateSecurityLayer)
@@ -1118,12 +1119,15 @@ void nego_enable_ext(rdpNego* nego, BOOL enable_ext)
  * @param RoutingTokenLength
  */
 
-void nego_set_routing_token(rdpNego* nego, BYTE* RoutingToken, DWORD RoutingTokenLength)
+BOOL nego_set_routing_token(rdpNego* nego, BYTE* RoutingToken, DWORD RoutingTokenLength)
 {
 	free(nego->RoutingToken);
 	nego->RoutingTokenLength = RoutingTokenLength;
 	nego->RoutingToken = (BYTE*) malloc(nego->RoutingTokenLength);
+	if (!nego->RoutingToken)
+		return FALSE;
 	CopyMemory(nego->RoutingToken, RoutingToken, nego->RoutingTokenLength);
+	return TRUE;
 }
 
 /**
@@ -1132,12 +1136,15 @@ void nego_set_routing_token(rdpNego* nego, BYTE* RoutingToken, DWORD RoutingToke
  * @param cookie
  */
 
-void nego_set_cookie(rdpNego* nego, char* cookie)
+BOOL nego_set_cookie(rdpNego* nego, char* cookie)
 {
 	if (nego->cookie)
 		free(nego->cookie);
 
 	nego->cookie = _strdup(cookie);
+	if (!nego->cookie)
+		return FALSE;
+	return TRUE;
 }
 
 /**

libfreerdp/core/nego.h

@@ -150,8 +150,8 @@ void nego_enable_rdp(rdpNego* nego, BOOL enable_rdp);
 void nego_enable_tls(rdpNego* nego, BOOL enable_tls);
 void nego_enable_nla(rdpNego* nego, BOOL enable_nla);
 void nego_enable_ext(rdpNego* nego, BOOL enable_ext);
-void nego_set_routing_token(rdpNego* nego, BYTE* RoutingToken, DWORD RoutingTokenLength);
+BOOL nego_set_routing_token(rdpNego* nego, BYTE* RoutingToken, DWORD RoutingTokenLength);
-void nego_set_cookie(rdpNego* nego, char* cookie);
+BOOL nego_set_cookie(rdpNego* nego, char* cookie);
 void nego_set_cookie_max_length(rdpNego* nego, UINT32 cookie_max_length);
 void nego_set_send_preconnection_pdu(rdpNego* nego, BOOL send_pcpdu);
 void nego_set_preconnection_id(rdpNego* nego, UINT32 id);

libfreerdp/core/nla.c

@@ -1356,6 +1356,8 @@ LPTSTR credssp_make_spn(const char* ServiceClass, const char* hostname)
 	}
 
 	ServicePrincipalName = (LPTSTR) malloc(SpnLength * sizeof(TCHAR));
+	if (!ServicePrincipalName)
+		return NULL;
 
 	status = DsMakeSpn(ServiceClassX, hostnameX, NULL, 0, NULL, &SpnLength, ServicePrincipalName);
 

libfreerdp/core/rdp.c

@@ -1199,12 +1199,11 @@ rdpRdp* rdp_new(rdpContext* context)
 {
 	rdpRdp* rdp;
 	DWORD flags;
+	BOOL newSettings = FALSE;
 
-	rdp = (rdpRdp*) malloc(sizeof(rdpRdp));
+	rdp = (rdpRdp*) calloc(1, sizeof(rdpRdp));
-
+	if (!rdp)
-	if (rdp)
+		return NULL;
-	{
-		ZeroMemory(rdp, sizeof(rdpRdp));
 
 	rdp->context = context;
 	rdp->instance = context->instance;
@@ -1215,31 +1214,103 @@ rdpRdp* rdp_new(rdpContext* context)
 		flags |= FREERDP_SETTINGS_SERVER_MODE;
 
 	if (!context->settings)
+	{
 		context->settings = freerdp_settings_new(flags);
+		if (!context->settings)
+			goto out_free;
+		newSettings = TRUE;
+	}
 
 	rdp->settings = context->settings;
 	rdp->settings->instance = context->instance;
-
 	if (context->instance)
 		context->instance->settings = rdp->settings;
 
 	rdp->extension = extension_new(context->instance);
+	if (!rdp->extension)
+		goto out_free_settings;
+
 	rdp->transport = transport_new(rdp->settings);
+	if (!rdp->transport)
+		goto out_free_extension;
 	rdp->transport->rdp = rdp;
+
 	rdp->license = license_new(rdp);
+	if (!rdp->license)
+		goto out_free_transport;
+
 	rdp->input = input_new(rdp);
+	if (!rdp->input)
+		goto out_free_license;
+
 	rdp->update = update_new(rdp);
+	if (!rdp->update)
+		goto out_free_input;
+
 	rdp->fastpath = fastpath_new(rdp);
+	if (!rdp->fastpath)
+		goto out_free_update;
+
 	rdp->nego = nego_new(rdp->transport);
+	if (!rdp->nego)
+		goto out_free_fastpath;
+
 	rdp->mcs = mcs_new(rdp->transport);
+	if (!rdp->mcs)
+		goto out_free_nego;
+
 	rdp->redirection = redirection_new();
+	if (!rdp->redirection)
+		goto out_free_mcs;
+
 	rdp->autodetect = autodetect_new();
+	if (!rdp->autodetect)
+		goto out_free_redirection;
+
 	rdp->heartbeat = heartbeat_new();
+	if (!rdp->heartbeat)
+		goto out_free_autodetect;
+
 	rdp->multitransport = multitransport_new();
+	if (!rdp->multitransport)
+		goto out_free_heartbeat;
+
 	rdp->bulk = bulk_new(context);
-	}
+	if (!rdp->bulk)
+		goto out_free_multitransport;
 
 	return rdp;
+
+out_free_multitransport:
+	multitransport_free(rdp->multitransport);
+out_free_heartbeat:
+	heartbeat_free(rdp->heartbeat);
+out_free_autodetect:
+	autodetect_free(rdp->autodetect);
+out_free_redirection:
+	redirection_free(rdp->redirection);
+out_free_mcs:
+	mcs_free(rdp->mcs);
+out_free_nego:
+	nego_free(rdp->nego);
+out_free_fastpath:
+	fastpath_free(rdp->fastpath);
+out_free_update:
+	update_free(rdp->update);
+out_free_input:
+	input_free(rdp->input);
+out_free_license:
+	license_free(rdp->license);
+out_free_transport:
+	transport_free(rdp->transport);
+out_free_extension:
+	extension_free(rdp->extension);
+out_free_settings:
+	if (newSettings)
+		freerdp_settings_free(rdp->settings);
+out_free:
+	free(rdp);
+	return NULL;
 }
 
 void rdp_reset(rdpRdp* rdp)