CVE-2020-6018: Out-of-bounds Write (CWE-787)
value: Valve's Game Networking Sockets prior to version v1.2.0 improperly handles long encrypted messages in function AES_GCM_DecryptContext::Decrypt() when compiled using libsodium, leading to a Stack-Based Buffer Overflow and resulting in a memory corruption and possibly even a remote code execution.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2020-12-02T01:15Z
Repository: https://github.com/ValveSoftware/GameNetworkingSockets
Language: C++
Files changed: src/common/crypto_libsodium.cpp
Commit date: 2020-09-03 15:05:55 -0700
Commit message: Check if output buffer is too small.

It really seems like libsodium (whose entire purpose is to make crypto
idiot-proof) making me mess with these details is a flaw in the API design.

Also, correct Hungarian.

Diff: diff --git a/src/common/crypto_libsodium.cpp b/src/common/crypto_libsodium.cpp
index 7641399..483e985 100644
--- a/src/common/crypto_libsodium.cpp
+++ b/src/common/crypto_libsodium.cpp
@@ -48,43 +48,56 @@ bool AES_GCM_EncryptContext::Encrypt(
 	const void *pPlaintextData, size_t cbPlaintextData,
 	const void *pIV,
 	void *pEncryptedDataAndTag, uint32 *pcbEncryptedDataAndTag,
 	const void *pAdditionalAuthenticationData, size_t cbAuthenticationData
 ) {
-    unsigned long long pcbEncryptedDataAndTag_longlong = *pcbEncryptedDataAndTag;
 
+	// Make sure caller's buffer is big enough to hold the result.
+	if ( cbPlaintextData + crypto_aead_aes256gcm_ABYTES > *pcbEncryptedDataAndTag )
+	{
+		*pcbEncryptedDataAndTag = 0;
+		return false;
+	}
+
+    unsigned long long cbEncryptedDataAndTag_longlong;
     crypto_aead_aes256gcm_encrypt_afternm(
-		static_cast<unsigned char*>( pEncryptedDataAndTag ), &pcbEncryptedDataAndTag_longlong,
+		static_cast<unsigned char*>( pEncryptedDataAndTag ), &cbEncryptedDataAndTag_longlong,
 		static_cast<const unsigned char*>( pPlaintextData ), cbPlaintextData,
 		static_cast<const unsigned char*>(pAdditionalAuthenticationData), cbAuthenticationData,
 		nullptr,
 		static_cast<const unsigned char*>( pIV ),
 		static_cast<const crypto_aead_aes256gcm_state*>( m_ctx )
 	);
 
-    *pcbEncryptedDataAndTag = pcbEncryptedDataAndTag_longlong;
+    *pcbEncryptedDataAndTag = cbEncryptedDataAndTag_longlong;
 
     return true;
 }
 
 bool AES_GCM_DecryptContext::Decrypt(
 	const void *pEncryptedDataAndTag, size_t cbEncryptedDataAndTag,
 	const void *pIV,
 	void *pPlaintextData, uint32 *pcbPlaintextData,
 	const void *pAdditionalAuthenticationData, size_t cbAuthenticationData
 ) {
-    unsigned long long pcbPlaintextData_longlong;
-
+	// Make sure caller's buffer is big enough to hold the result
+	if ( cbEncryptedDataAndTag > *pcbPlaintextData + crypto_aead_aes256gcm_ABYTES )
+	{
+		*pcbPlaintextData = 0;
+		return false;
+	}
+
+    unsigned long long cbPlaintextData_longlong;
     const int nDecryptResult = crypto_aead_aes256gcm_decrypt_afternm(
-		static_cast<unsigned char*>( pPlaintextData ), &pcbPlaintextData_longlong,
+		static_cast<unsigned char*>( pPlaintextData ), &cbPlaintextData_longlong,
 		nullptr,
 		static_cast<const unsigned char*>( pEncryptedDataAndTag ), cbEncryptedDataAndTag,
 		static_cast<const unsigned char*>( pAdditionalAuthenticationData ), cbAuthenticationData,
 		static_cast<const unsigned char*>( pIV ), static_cast<const crypto_aead_aes256gcm_state*>( m_ctx )
 	);
 
-    *pcbPlaintextData = pcbPlaintextData_longlong;
+    *pcbPlaintextData = cbPlaintextData_longlong;
 
     return nDecryptResult == 0;
 }
 
 void CCrypto::Init()

Vulnerable code:     unsigned long long pcbEncryptedDataAndTag_longlong = *pcbEncryptedDataAndTag;
		static_cast<unsigned char*>( pEncryptedDataAndTag ), &pcbEncryptedDataAndTag_longlong,
    *pcbEncryptedDataAndTag = pcbEncryptedDataAndTag_longlong;
    unsigned long long pcbPlaintextData_longlong;

		static_cast<unsigned char*>( pPlaintextData ), &pcbPlaintextData_longlong,
    *pcbPlaintextData = pcbPlaintextData_longlong;
Fixed code: 	// Make sure caller's buffer is big enough to hold the result.
	if ( cbPlaintextData + crypto_aead_aes256gcm_ABYTES > *pcbEncryptedDataAndTag )
	{
		*pcbEncryptedDataAndTag = 0;
		return false;
	}

    unsigned long long cbEncryptedDataAndTag_longlong;
		static_cast<unsigned char*>( pEncryptedDataAndTag ), &cbEncryptedDataAndTag_longlong,
    *pcbEncryptedDataAndTag = cbEncryptedDataAndTag_longlong;
	// Make sure caller's buffer is big enough to hold the result
	if ( cbEncryptedDataAndTag > *pcbPlaintextData + crypto_aead_aes256gcm_ABYTES )
	{
		*pcbPlaintextData = 0;
		return false;
	}

    unsigned long long cbPlaintextData_longlong;
		static_cast<unsigned char*>( pPlaintextData ), &cbPlaintextData_longlong,
    *pcbPlaintextData = cbPlaintextData_longlong;