Summary

An out-of-bounds (OOB) write vulnerability has been detected in FreeRDP’s crypto_rsa_common (heap overflow).

Product

FreeRDP

Tested Version

Development version - master branch (May 19, 2020)

Details: Out-of-bound write in crypto_rsa_common

The crypto_rsa_common function in crypto.c performs the following call to memcpy:

crypto.c:crypto_rsa_common:
...
static int crypto_rsa_common(const BYTE* input, int length, UINT32 key_length, const BYTE* modulus,
                             const BYTE* exponent, int exponent_size, BYTE* output)
{
...
memcpy(input_reverse, input, length)
...

The input_reverse buffer is allocated as:

input_reverse = (BYTE*)malloc(2 * key_length + exponent_size);

So we notice a classic pattern in which there’s a disjointed relationship between an integer that influences the POPULATION of a memory region (length) and an integer that influences the ALLOCATION of a memory region (key_length).

While in many of the paths to this code there exists some loose sanity checking logic that is supposed to ensure there is a logical relationship between these two variables, this logic can often be circumvented to subsequently trigger heap overflow due to an underallocation based on control of the key_length variable.

There exist multiple paths to trigger this scenario throughout the codebase since crypto_rsa_common is used as the underlying API for most all encrypt/decrypt operations, and many of them may be influenced by remotely controlled input.

For example:

BOOL license_encrypt_premaster_secret(rdpLicense* license)
{
	BYTE* EncryptedPremasterSecret;
	if (!license_get_server_rsa_public_key(license))
		return FALSE;
#ifdef WITH_DEBUG_LICENSE
	WLog_DBG(TAG, "Modulus (%" PRIu32 " bits):", license->ModulusLength * 8);
	winpr_HexDump(TAG, WLOG_DEBUG, license->Modulus, license->ModulusLength);
	WLog_DBG(TAG, "Exponent:");
	winpr_HexDump(TAG, WLOG_DEBUG, license->Exponent, 4);
#endif
	EncryptedPremasterSecret = (BYTE*)calloc(1, license->ModulusLength);
	if (!EncryptedPremasterSecret)
		return FALSE;
	license->EncryptedPremasterSecret->type = BB_RANDOM_BLOB;
	license->EncryptedPremasterSecret->length = PREMASTER_SECRET_LENGTH;
#ifndef LICENSE_NULL_PREMASTER_SECRET
	license->EncryptedPremasterSecret->length = crypto_rsa_public_encrypt(
	    license->PremasterSecret, PREMASTER_SECRET_LENGTH, license->ModulusLength, license->Modulus,
	    license->Exponent, EncryptedPremasterSecret);

In this code path crypto_rsa_public_encrypt calls through to crypto_rsa_common. We note that the length variable is set to a static value PREMASTER_SECRET_LENGTH (48), yet the key_length variable is set via the server-side public key controlled license->ModulusLength, so in this case any key_length value that results in an allocation smaller than 48 bytes would be sufficient to trigger heap overflow.

Impact

This issue may lead to remote heap overflow and potentially Remote Code Execution (RCE).

CVE

Coordinated Disclosure Timeline

Resources

Credit

This issue was discovered and reported by GHSL team member @antonio-morales (Antonio Morales).

Contact

You can contact the GHSL team at securitylab@github.com, please include GHSL-2020-102 in any communication regarding this issue.