Skip to content

Commit

Permalink
Fix multiple security issues
Browse files Browse the repository at this point in the history
Fix the following issues identified by the CISCO TALOS project:

* TALOS-2017-0336 CVE-2017-2834
* TALOS-2017-0337 CVE-2017-2835
* TALOS-2017-0338 CVE-2017-2836
* TALOS-2017-0339 CVE-2017-2837
* TALOS-2017-0340 CVE-2017-2838
* TALOS-2017-0341 CVE-2017-2839

Backported based on commit 8292b45.
  • Loading branch information
bmiklautz committed Jul 27, 2017
1 parent 4c69c3e commit 03ab683
Show file tree
Hide file tree
Showing 18 changed files with 174 additions and 96 deletions.
4 changes: 2 additions & 2 deletions libfreerdp/core/capabilities.c
Original file line number Diff line number Diff line change
Expand Up @@ -3341,12 +3341,12 @@ BOOL rdp_recv_get_active_header(rdpRdp* rdp, wStream* s, UINT16* pChannelId)

if (rdp->settings->DisableEncryption)
{
if (!rdp_read_security_header(s, &securityFlags))
if (!rdp_read_security_header(s, &securityFlags, &length))
return FALSE;

if (securityFlags & SEC_ENCRYPT)
{
if (!rdp_decrypt(rdp, s, length - 4, securityFlags))
if (!rdp_decrypt(rdp, s, length, securityFlags))
{
fprintf(stderr, "rdp_decrypt failed\n");
return FALSE;
Expand Down
18 changes: 11 additions & 7 deletions libfreerdp/core/certificate.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,10 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w
UINT32 keylen;
UINT32 bitlen;
UINT32 datalen;
UINT32 modlen;

if (Stream_GetRemainingLength(s) < 20)
return FALSE;

Stream_Read(s, magic, 4);

if (memcmp(magic, "RSA1", 4) != 0)
Expand All @@ -343,12 +343,16 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w
Stream_Read_UINT32(s, bitlen);
Stream_Read_UINT32(s, datalen);
Stream_Read(s, certificate->cert_info.exponent, 4);
modlen = keylen - 8;

if (Stream_GetRemainingLength(s) < modlen + 8) // count padding
if ((keylen <= 8) || (Stream_GetRemainingLength(s) < keylen))
return FALSE;
certificate->cert_info.ModulusLength = modlen;

certificate->cert_info.ModulusLength = keylen - 8;
certificate->cert_info.Modulus = malloc(certificate->cert_info.ModulusLength);

if (!certificate->cert_info.Modulus)
return FALSE;

Stream_Read(s, certificate->cert_info.Modulus, certificate->cert_info.ModulusLength);
/* 8 bytes of zero padding */
Stream_Seek(s, 8);
Expand Down Expand Up @@ -500,7 +504,7 @@ BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate

BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, wStream* s)
{
int i;
UINT32 i;
UINT32 certLength;
UINT32 numCertBlobs;
BOOL ret;
Expand All @@ -513,7 +517,7 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate,

certificate->x509_cert_chain = certificate_new_x509_certificate_chain(numCertBlobs);

for (i = 0; i < (int) numCertBlobs; i++)
for (i = 0; i < numCertBlobs; i++)
{
if (Stream_GetRemainingLength(s) < 4)
return FALSE;
Expand Down Expand Up @@ -562,7 +566,7 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate,
* @param length certificate length
*/

int certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, int length)
int certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, size_t length)
{
wStream* s;
UINT32 dwVersion;
Expand Down
2 changes: 1 addition & 1 deletion libfreerdp/core/certificate.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void certificate_free_x509_certificate_chain(rdpX509CertChain* x509_cert_chain);

BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate, wStream* s);
BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, wStream* s);
int certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, int length);
int certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, size_t length);

rdpCertificate* certificate_new(void);
void certificate_free(rdpCertificate* certificate);
Expand Down
17 changes: 8 additions & 9 deletions libfreerdp/core/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,18 @@ BOOL rdp_client_connect(rdpRdp* rdp)

if (settings->GatewayEnabled)
{
char* user;
char* user = NULL;
char* domain;
char* cookie;
int user_length = 0;
int user_length = 0;
int domain_length;
int cookie_length;


if (settings->Username)
{
user = settings->Username;
user_length = strlen(settings->Username);
}
if (settings->Username)
{
user = settings->Username;
user_length = strlen(settings->Username);
}

if (settings->Domain)
domain = settings->Domain;
Expand Down Expand Up @@ -365,7 +364,7 @@ static BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s)
return FALSE;
}

if (!rdp_read_security_header(s, &sec_flags))
if (!rdp_read_security_header(s, &sec_flags, NULL))
return FALSE;

if ((sec_flags & SEC_EXCHANGE_PKT) == 0)
Expand Down
60 changes: 34 additions & 26 deletions libfreerdp/core/gcc.c
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ BOOL gcc_read_server_security_data(wStream* s, rdpSettings* settings)

if (Stream_GetRemainingLength(s) < 8)
return FALSE;

Stream_Read_UINT32(s, settings->EncryptionMethods); /* encryptionMethod */
Stream_Read_UINT32(s, settings->EncryptionLevel); /* encryptionLevel */

Expand All @@ -844,43 +845,50 @@ BOOL gcc_read_server_security_data(wStream* s, rdpSettings* settings)

if (Stream_GetRemainingLength(s) < 8)
return FALSE;

Stream_Read_UINT32(s, settings->ServerRandomLength); /* serverRandomLen */
Stream_Read_UINT32(s, settings->ServerCertificateLength); /* serverCertLen */

if (Stream_GetRemainingLength(s) < settings->ServerRandomLength + settings->ServerCertificateLength)
if (settings->ServerRandomLength == 0 || settings->ServerCertificateLength == 0)
return FALSE;

if (settings->ServerRandomLength > 0)
{
/* serverRandom */
settings->ServerRandom = (BYTE*) malloc(settings->ServerRandomLength);
Stream_Read(s, settings->ServerRandom, settings->ServerRandomLength);
}
else
{
if (Stream_GetRemainingLength(s) < settings->ServerRandomLength)
return FALSE;
}

if (settings->ServerCertificateLength > 0)
{
/* serverCertificate */
settings->ServerCertificate = (BYTE*) malloc(settings->ServerCertificateLength);
Stream_Read(s, settings->ServerCertificate, settings->ServerCertificateLength);
/* serverRandom */
settings->ServerRandom = (BYTE*) malloc(settings->ServerRandomLength);
if (!settings->ServerRandom)
return FALSE;
Stream_Read(s, settings->ServerRandom, settings->ServerRandomLength);

certificate_free(settings->RdpServerCertificate);
settings->RdpServerCertificate = certificate_new();
data = settings->ServerCertificate;
length = settings->ServerCertificateLength;
/* serverCertificate */
if(Stream_GetRemainingLength(s) < settings->ServerCertificateLength)
goto out_fail1;
settings->ServerCertificate = (BYTE*) malloc(settings->ServerCertificateLength);
if (!settings->ServerCertificate)
goto out_fail1;

if (certificate_read_server_certificate(settings->RdpServerCertificate, data, length) < 1)
return FALSE;
}
else
{
return FALSE;
}
Stream_Read(s, settings->ServerCertificate, settings->ServerCertificateLength);
certificate_free(settings->RdpServerCertificate);
settings->RdpServerCertificate = certificate_new();
if (!settings->RdpServerCertificate)
goto out_fail2;

data = settings->ServerCertificate;
length = settings->ServerCertificateLength;

if (certificate_read_server_certificate(settings->RdpServerCertificate, data, length) < 1)
goto out_fail2;

return TRUE;

out_fail2:
free(settings->ServerCertificate);
settings->ServerCertificate = NULL;
out_fail1:
free(settings->ServerRandom);
settings->ServerRandom = NULL;
return FALSE;
}

static const BYTE initial_signature[] =
Expand Down
4 changes: 2 additions & 2 deletions libfreerdp/core/info.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ BOOL rdp_recv_client_info(rdpRdp* rdp, wStream* s)
if (!rdp_read_header(rdp, s, &length, &channelId))
return FALSE;

if (!rdp_read_security_header(s, &securityFlags))
if (!rdp_read_security_header(s, &securityFlags, &length))
return FALSE;

if ((securityFlags & SEC_INFO_PKT) == 0)
Expand All @@ -457,7 +457,7 @@ BOOL rdp_recv_client_info(rdpRdp* rdp, wStream* s)

if (securityFlags & SEC_ENCRYPT)
{
if (!rdp_decrypt(rdp, s, length - 4, securityFlags))
if (!rdp_decrypt(rdp, s, length, securityFlags))
{
fprintf(stderr, "rdp_decrypt failed\n");
return FALSE;
Expand Down
39 changes: 29 additions & 10 deletions libfreerdp/core/license.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@ BOOL license_recv(rdpLicense* license, wStream* s)
return FALSE;
}

if (!rdp_read_security_header(s, &securityFlags))
if (!rdp_read_security_header(s, &securityFlags, &length))
return FALSE;

if (securityFlags & SEC_ENCRYPT)
{
if (!rdp_decrypt(license->rdp, s, length - 4, securityFlags))
if (!rdp_decrypt(license->rdp, s, length, securityFlags))
{
fprintf(stderr, "rdp_decrypt failed\n");
return FALSE;
Expand Down Expand Up @@ -474,25 +474,41 @@ BOOL license_read_product_info(wStream* s, PRODUCT_INFO* productInfo)

Stream_Read_UINT32(s, productInfo->cbCompanyName); /* cbCompanyName (4 bytes) */

if (Stream_GetRemainingLength(s) < productInfo->cbCompanyName + 4)
/* Name must be > 0, but there is no upper limit defined, use UINT32_MAX */
if ((productInfo->cbCompanyName < 2) || (productInfo->cbCompanyName % 2 != 0))
return FALSE;

if (Stream_GetRemainingLength(s) < productInfo->cbCompanyName)
return FALSE;

productInfo->pbCompanyName = (BYTE*) malloc(productInfo->cbCompanyName);
if (!productInfo->pbCompanyName)
return FALSE;
Stream_Read(s, productInfo->pbCompanyName, productInfo->cbCompanyName);

if (Stream_GetRemainingLength(s) < 4)
goto out_fail;

Stream_Read_UINT32(s, productInfo->cbProductId); /* cbProductId (4 bytes) */

if ((productInfo->cbProductId < 2) || (productInfo->cbProductId % 2 != 0))
goto out_fail;

if (Stream_GetRemainingLength(s) < productInfo->cbProductId)
{
free(productInfo->pbCompanyName);
productInfo->pbCompanyName = NULL;
return FALSE;
}
goto out_fail;

productInfo->pbProductId = (BYTE*) malloc(productInfo->cbProductId);
Stream_Read(s, productInfo->pbProductId, productInfo->cbProductId);
if (!productInfo->pbProductId)
goto out_fail;

Stream_Read(s, productInfo->pbProductId, productInfo->cbProductId);
return TRUE;

out_fail:
free(productInfo->pbCompanyName);
productInfo->pbCompanyName = NULL;
return FALSE;

}

/**
Expand Down Expand Up @@ -796,7 +812,10 @@ BOOL license_read_platform_challenge_packet(rdpLicense* license, wStream* s)

/* EncryptedPlatformChallenge */
license->EncryptedPlatformChallenge->type = BB_ANY_BLOB;
license_read_binary_blob(s, license->EncryptedPlatformChallenge);

if (!license_read_binary_blob(s, license->EncryptedPlatformChallenge))
return FALSE;

license->EncryptedPlatformChallenge->type = BB_ENCRYPTED_DATA_BLOB;

if (Stream_GetRemainingLength(s) < 16)
Expand Down
17 changes: 14 additions & 3 deletions libfreerdp/core/mcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ BOOL mcs_read_domain_mcspdu_header(wStream* s, enum DomainMCSPDU* domainMCSPDU,
BYTE choice;
enum DomainMCSPDU MCSPDU;

*length = tpkt_read_header(s);
if (!tpkt_read_header(s, length))
return FALSE;

if (!tpdu_read_data(s, &li))
return FALSE;
Expand Down Expand Up @@ -332,8 +333,13 @@ BOOL mcs_recv_connect_initial(rdpMcs* mcs, wStream* s)
UINT16 li;
int length;
BOOL upwardFlag;
UINT16 tlength;

if (!mcs || !s)
return FALSE;

tpkt_read_header(s);
if (!tpkt_read_header(s, &tlength))
return FALSE;

if (!tpdu_read_data(s, &li))
return FALSE;
Expand Down Expand Up @@ -504,8 +510,13 @@ BOOL mcs_recv_connect_response(rdpMcs* mcs, wStream* s)
BYTE result;
UINT16 li;
UINT32 calledConnectId;
UINT16 tlength;

tpkt_read_header(s);
if (!mcs || !s)
return FALSE;

if (!tpkt_read_header(s, &tlength))
return FALSE;

if (!tpdu_read_data(s, &li))
return FALSE;
Expand Down
8 changes: 4 additions & 4 deletions libfreerdp/core/nego.c
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,7 @@ int nego_recv(rdpTransport* transport, wStream* s, void* extra)
UINT16 length;
rdpNego* nego = (rdpNego*) extra;

length = tpkt_read_header(s);

if (length == 0)
if (!tpkt_read_header(s, &length) || length == 0)
return -1;

if (!tpdu_read_connection_confirm(s, &li))
Expand Down Expand Up @@ -582,8 +580,10 @@ BOOL nego_read_request(rdpNego* nego, wStream* s)
BYTE li;
BYTE c;
BYTE type;
UINT16 length;

tpkt_read_header(s);
if (!tpkt_read_header(s, &length))
return FALSE;

if (!tpdu_read_connection_request(s, &li))
return FALSE;
Expand Down
4 changes: 2 additions & 2 deletions libfreerdp/core/peer.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ static int peer_recv_tpkt_pdu(freerdp_peer* client, wStream* s)

if (rdp->settings->DisableEncryption)
{
if (!rdp_read_security_header(s, &securityFlags))
if (!rdp_read_security_header(s, &securityFlags, &length))
return -1;

if (securityFlags & SEC_ENCRYPT)
{
if (!rdp_decrypt(rdp, s, length - 4, securityFlags))
if (!rdp_decrypt(rdp, s, length, securityFlags))
{
fprintf(stderr, "rdp_decrypt failed\n");
return -1;
Expand Down

0 comments on commit 03ab683

Please sign in to comment.