From 2e87d0ee522acb15818c06d52112552d4010b21b Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Tue, 23 Jun 2015 15:40:37 +0200 Subject: [PATCH 1/2] Fix leaks in certificate and identity handling --- libfreerdp/core/nla.c | 24 +++++++++++++++--------- libfreerdp/crypto/certificate.c | 1 + libfreerdp/crypto/tls.c | 8 ++------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/libfreerdp/core/nla.c b/libfreerdp/core/nla.c index df9d61c30..ecd2e57a1 100644 --- a/libfreerdp/core/nla.c +++ b/libfreerdp/core/nla.c @@ -99,10 +99,23 @@ static SECURITY_STATUS nla_decrypt_public_key_echo(rdpNla* nla); static SECURITY_STATUS nla_encrypt_ts_credentials(rdpNla* nla); static SECURITY_STATUS nla_decrypt_ts_credentials(rdpNla* nla); static BOOL nla_read_ts_password_creds(rdpNla* nla, wStream* s); +static void nla_identity_free(SEC_WINNT_AUTH_IDENTITY* identity); #define ber_sizeof_sequence_octet_string(length) ber_sizeof_contextual_tag(ber_sizeof_octet_string(length)) + ber_sizeof_octet_string(length) #define ber_write_sequence_octet_string(stream, context, value, length) ber_write_contextual_tag(stream, context, ber_sizeof_octet_string(length), TRUE) + ber_write_octet_string(stream, value, length) +void nla_identity_free(SEC_WINNT_AUTH_IDENTITY* identity) +{ + if (identity) + { + free(identity->User); + free(identity->Domain); + free(identity->Password); + } + free(identity); + +} + /** * Initialize NTLMSSP authentication module (client). * @param credssp @@ -156,7 +169,7 @@ int nla_client_init(rdpNla* nla) if (!settings->Username) { - free (nla->identity); + nla_identity_free(nla->identity); nla->identity = NULL; } else @@ -1561,13 +1574,6 @@ void nla_free(rdpNla* nla) sspi_SecBufferFree(&nla->tsCredentials); free(nla->ServicePrincipalName); - if (nla->identity) - { - free(nla->identity->User); - free(nla->identity->Domain); - free(nla->identity->Password); - } - free(nla->identity); - + nla_identity_free(nla->identity); free(nla); } diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index bfe8e3412..008c29c2d 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -543,6 +543,7 @@ void certificate_store_free(rdpCertificateStore* certstore) { free(certstore->path); free(certstore->file); + free(certstore->legacy_file); free(certstore); } } diff --git a/libfreerdp/crypto/tls.c b/libfreerdp/crypto/tls.c index 082df9656..b33bb5145 100644 --- a/libfreerdp/crypto/tls.c +++ b/libfreerdp/crypto/tls.c @@ -1089,6 +1089,7 @@ int tls_verify_certificate(rdpTls* tls, CryptoCert cert, char* hostname, int por /* verify certificate name match */ certificate_data = crypto_get_certificate_data(cert->px509, hostname, port); + /* extra common name and alternative names */ common_name = crypto_cert_subject_common_name(cert->px509, &common_name_length); alt_names = crypto_cert_subject_alt_name(cert->px509, &alt_names_count, &alt_names_lengths); @@ -1222,12 +1223,7 @@ int tls_verify_certificate(rdpTls* tls, CryptoCert cert, char* hostname, int por free(fingerprint); } - if (certificate_data) - { - free(certificate_data->fingerprint); - free(certificate_data->hostname); - free(certificate_data); - } + certificate_data_free(certificate_data); #ifndef _WIN32 free(common_name); From 77ef5a80de26869b5916218f859960bdda3cd867 Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Fri, 26 Jun 2015 15:12:33 +0200 Subject: [PATCH 2/2] nla: clear identity memory before releasing --- libfreerdp/core/nla.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/libfreerdp/core/nla.c b/libfreerdp/core/nla.c index ecd2e57a1..5174502a8 100644 --- a/libfreerdp/core/nla.c +++ b/libfreerdp/core/nla.c @@ -108,9 +108,21 @@ void nla_identity_free(SEC_WINNT_AUTH_IDENTITY* identity) { if (identity) { - free(identity->User); - free(identity->Domain); - free(identity->Password); + if (identity->User) + { + memset(identity->User, 0, identity->UserLength*2); + free(identity->User); + } + if (identity->Password) + { + memset(identity->Password, 0, identity->PasswordLength*2); + free(identity->Password); + } + if (identity->Domain) + { + memset(identity->Domain, 0, identity->DomainLength*2); + free(identity->Domain); + } } free(identity);