From a1f2c1e161e96e56d7bb000cfcc737943cad3f5a Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Mon, 11 May 2020 08:50:15 +0200
Subject: [PATCH] Fixed #6156: Enforce synchronized encrypt count

Old style RDP encryption uses a counter, synchronize this for
packets send from different threads.
---
 libfreerdp/core/rdp.c      | 58 +++++++++++---------------------------
 libfreerdp/core/rdp.h      |  1 +
 libfreerdp/core/security.c | 22 +++++++++++----
 3 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c
index b9c60c74e..fd771ae3e 100644
--- a/libfreerdp/core/rdp.c
+++ b/libfreerdp/core/rdp.c
@@ -1751,6 +1751,7 @@ rdpRdp* rdp_new(rdpContext* context)
 	if (!rdp)
 		return NULL;
 
+	InitializeCriticalSection(&rdp->critical);
 	rdp->context = context;
 	rdp->instance = context->instance;
 	flags = 0;
@@ -1763,7 +1764,7 @@ rdpRdp* rdp_new(rdpContext* context)
 		context->settings = freerdp_settings_new(flags);
 
 		if (!context->settings)
-			goto out_free;
+			goto fail;
 
 		newSettings = TRUE;
 	}
@@ -1784,93 +1785,67 @@ rdpRdp* rdp_new(rdpContext* context)
 	rdp->transport = transport_new(context);
 
 	if (!rdp->transport)
-		goto out_free_settings;
+		goto fail;
 
 	rdp->license = license_new(rdp);
 
 	if (!rdp->license)
-		goto out_free_transport;
+		goto fail;
 
 	rdp->input = input_new(rdp);
 
 	if (!rdp->input)
-		goto out_free_license;
+		goto fail;
 
 	rdp->update = update_new(rdp);
 
 	if (!rdp->update)
-		goto out_free_input;
+		goto fail;
 
 	rdp->fastpath = fastpath_new(rdp);
 
 	if (!rdp->fastpath)
-		goto out_free_update;
+		goto fail;
 
 	rdp->nego = nego_new(rdp->transport);
 
 	if (!rdp->nego)
-		goto out_free_fastpath;
+		goto fail;
 
 	rdp->mcs = mcs_new(rdp->transport);
 
 	if (!rdp->mcs)
-		goto out_free_nego;
+		goto fail;
 
 	rdp->redirection = redirection_new();
 
 	if (!rdp->redirection)
-		goto out_free_mcs;
+		goto fail;
 
 	rdp->autodetect = autodetect_new();
 
 	if (!rdp->autodetect)
-		goto out_free_redirection;
+		goto fail;
 
 	rdp->heartbeat = heartbeat_new();
 
 	if (!rdp->heartbeat)
-		goto out_free_autodetect;
+		goto fail;
 
 	rdp->multitransport = multitransport_new();
 
 	if (!rdp->multitransport)
-		goto out_free_heartbeat;
+		goto fail;
 
 	rdp->bulk = bulk_new(context);
 
 	if (!rdp->bulk)
-		goto out_free_multitransport;
+		goto fail;
 
 	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_settings:
 
-	if (newSettings)
-		freerdp_settings_free(rdp->settings);
-
-out_free:
-	free(rdp);
+fail:
+	rdp_free(rdp);
 	return NULL;
 }
 
@@ -1950,6 +1925,7 @@ void rdp_free(rdpRdp* rdp)
 {
 	if (rdp)
 	{
+		DeleteCriticalSection(&rdp->critical);
 		winpr_RC4_Free(rdp->rc4_decrypt_key);
 		winpr_RC4_Free(rdp->rc4_encrypt_key);
 		winpr_Cipher_Free(rdp->fips_encrypt);
diff --git a/libfreerdp/core/rdp.h b/libfreerdp/core/rdp.h
index e728ea357..47281d611 100644
--- a/libfreerdp/core/rdp.h
+++ b/libfreerdp/core/rdp.h
@@ -179,6 +179,7 @@ struct rdp_rdp
 	UINT64 inPackets;
 	UINT64 outBytes;
 	UINT64 outPackets;
+	CRITICAL_SECTION critical;
 };
 
 FREERDP_LOCAL BOOL rdp_read_security_header(wStream* s, UINT16* flags, UINT16* length);
diff --git a/libfreerdp/core/security.c b/libfreerdp/core/security.c
index 9ef9b1afd..bce1855c4 100644
--- a/libfreerdp/core/security.c
+++ b/libfreerdp/core/security.c
@@ -712,26 +712,31 @@ out:
 
 BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp)
 {
+	BOOL rc = FALSE;
+	EnterCriticalSection(&rdp->critical);
 	if (rdp->encrypt_use_count >= 4096)
 	{
 		if (!security_key_update(rdp->encrypt_key, rdp->encrypt_update_key, rdp->rc4_key_len, rdp))
-			return FALSE;
+			goto fail;
 
 		winpr_RC4_Free(rdp->rc4_encrypt_key);
 		rdp->rc4_encrypt_key = winpr_RC4_New(rdp->encrypt_key, rdp->rc4_key_len);
 
 		if (!rdp->rc4_encrypt_key)
-			return FALSE;
+			goto fail;
 
 		rdp->encrypt_use_count = 0;
 	}
 
 	if (!winpr_RC4_Update(rdp->rc4_encrypt_key, length, data, data))
-		return FALSE;
+		goto fail;
 
 	rdp->encrypt_use_count++;
 	rdp->encrypt_checksum_use_count++;
-	return TRUE;
+	rc = TRUE;
+fail:
+	LeaveCriticalSection(&rdp->critical);
+	return rc;
 }
 
 BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp)
@@ -793,13 +798,18 @@ out:
 
 BOOL security_fips_encrypt(BYTE* data, size_t length, rdpRdp* rdp)
 {
+	BOOL rc = FALSE;
 	size_t olen;
 
+	EnterCriticalSection(&rdp->critical);
 	if (!winpr_Cipher_Update(rdp->fips_encrypt, data, length, data, &olen))
-		return FALSE;
+		goto fail;
 
 	rdp->encrypt_use_count++;
-	return TRUE;
+	rc = TRUE;
+fail:
+	LeaveCriticalSection(&rdp->critical);
+	return rc;
 }
 
 BOOL security_fips_decrypt(BYTE* data, size_t length, rdpRdp* rdp)