From ac7507ab8dcf6891c73f245f6481e5473eef574d Mon Sep 17 00:00:00 2001 From: Hardening Date: Tue, 25 Mar 2014 23:13:08 +0100 Subject: [PATCH] Adds some check to treat OOM problems + RDP security fix Malloc can fail so it will, this patch adds some check in some places where malloc/strdup results were not checked. This patch also contains a server side fix for RDP security (credit to nfedera). The signature len was badly set in the GCC packet. And some other RDP security oriented fixes are also there. --- libfreerdp/common/settings.c | 115 +++++++++++++++++++++++++---- libfreerdp/core/bulk.c | 14 ++-- libfreerdp/core/certificate.c | 156 ++++++++++++++++++++++------------------ libfreerdp/core/connection.c | 111 ++++++++++++++++++++-------- libfreerdp/core/gcc.c | 8 ++- libfreerdp/core/info.c | 8 ++- libfreerdp/core/license.c | 16 +++++ libfreerdp/core/nego.c | 21 +++--- libfreerdp/core/peer.c | 25 ++++--- libfreerdp/core/security.c | 86 ++++++++++++++++++++++ libfreerdp/crypto/certificate.c | 26 ++++--- libfreerdp/crypto/crypto.c | 51 ++++++++++--- winpr/include/winpr/file.h | 2 +- 13 files changed, 480 insertions(+), 159 deletions(-) diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index d9b9a41..9029399 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -166,70 +166,133 @@ RDPDR_DEVICE* freerdp_device_collection_find(rdpSettings* settings, const char* RDPDR_DEVICE* freerdp_device_clone(RDPDR_DEVICE* device) { - RDPDR_DEVICE* _device = NULL; - if (device->Type == RDPDR_DTYP_FILESYSTEM) { RDPDR_DRIVE* drive = (RDPDR_DRIVE*) device; RDPDR_DRIVE* _drive = (RDPDR_DRIVE*) malloc(sizeof(RDPDR_DRIVE)); + if (!_drive) + return NULL; _drive->Id = drive->Id; _drive->Type = drive->Type; _drive->Name = _strdup(drive->Name); + if (!_drive->Name) + goto out_fs_name_error; _drive->Path = _strdup(drive->Path); + if (!_drive->Path) + goto out_fs_path_error; + + return (RDPDR_DEVICE*) _drive; - _device = (RDPDR_DEVICE*) _drive; + out_fs_path_error: + free(_drive->Name); + out_fs_name_error: + free(_drive); + return NULL; } - else if (device->Type == RDPDR_DTYP_PRINT) + + if (device->Type == RDPDR_DTYP_PRINT) { RDPDR_PRINTER* printer = (RDPDR_PRINTER*) device; RDPDR_PRINTER* _printer = (RDPDR_PRINTER*) malloc(sizeof(RDPDR_PRINTER)); + if (!_printer) + return NULL; _printer->Id = printer->Id; _printer->Type = printer->Type; _printer->Name = _strdup(printer->Name); + if (!_printer->Name) + goto out_print_name_error; _printer->DriverName = _strdup(printer->DriverName); + if(!_printer->DriverName) + goto out_print_path_error; + + return (RDPDR_DEVICE*) _printer; - _device = (RDPDR_DEVICE*) _printer; + out_print_path_error: + free(_printer->Name); + out_print_name_error: + free(_printer); + return NULL; } - else if (device->Type == RDPDR_DTYP_SMARTCARD) + + if (device->Type == RDPDR_DTYP_SMARTCARD) { RDPDR_SMARTCARD* smartcard = (RDPDR_SMARTCARD*) device; RDPDR_SMARTCARD* _smartcard = (RDPDR_SMARTCARD*) malloc(sizeof(RDPDR_SMARTCARD)); + if (!_smartcard) + return NULL; _smartcard->Id = smartcard->Id; _smartcard->Type = smartcard->Type; _smartcard->Name = _strdup(smartcard->Name); + if (!_smartcard->Name) + goto out_smartc_name_error; _smartcard->Path = _strdup(smartcard->Path); + if (!_smartcard->Path) + goto out_smartc_path_error; + + return (RDPDR_DEVICE*) _smartcard; - _device = (RDPDR_DEVICE*) _smartcard; + out_smartc_path_error: + free(_smartcard->Name); + out_smartc_name_error: + free(_smartcard); + return NULL; } - else if (device->Type == RDPDR_DTYP_SERIAL) + + if (device->Type == RDPDR_DTYP_SERIAL) { RDPDR_SERIAL* serial = (RDPDR_SERIAL*) device; RDPDR_SERIAL* _serial = (RDPDR_SERIAL*) malloc(sizeof(RDPDR_SERIAL)); + if (!_serial) + return NULL; _serial->Id = serial->Id; _serial->Type = serial->Type; _serial->Name = _strdup(serial->Name); + if (!_serial->Name) + goto out_serial_name_error; _serial->Path = _strdup(serial->Path); + if (!_serial->Path) + goto out_serial_path_error; - _device = (RDPDR_DEVICE*) _serial; + return (RDPDR_DEVICE*) _serial; + + out_serial_path_error: + free(_serial->Name); + out_serial_name_error: + free(_serial); + return NULL; } - else if (device->Type == RDPDR_DTYP_PARALLEL) + + if (device->Type == RDPDR_DTYP_PARALLEL) { RDPDR_PARALLEL* parallel = (RDPDR_PARALLEL*) device; RDPDR_PARALLEL* _parallel = (RDPDR_PARALLEL*) malloc(sizeof(RDPDR_PARALLEL)); + if (!_parallel) + return NULL; _parallel->Id = parallel->Id; _parallel->Type = parallel->Type; _parallel->Name = _strdup(parallel->Name); + if (!_parallel->Name) + goto out_parallel_name_error; _parallel->Path = _strdup(parallel->Path); + if (!_parallel->Path) + goto out_parallel_path_error; + + return (RDPDR_DEVICE*) _parallel; + out_parallel_path_error: + free(_parallel->Name); + out_parallel_name_error: + free(_parallel); + return NULL; - _device = (RDPDR_DEVICE*) _parallel; } - return _device; + fprintf(stderr, "%s: unknown device type %d\n", __FUNCTION__, device->Type); + return NULL; } void freerdp_device_collection_free(rdpSettings* settings) @@ -308,16 +371,29 @@ ADDIN_ARGV* freerdp_static_channel_clone(ADDIN_ARGV* channel) ADDIN_ARGV* _channel = NULL; _channel = (ADDIN_ARGV*) malloc(sizeof(ADDIN_ARGV)); + if (!_channel) + return NULL; _channel->argc = channel->argc; - _channel->argv = (char**) malloc(sizeof(char*) * channel->argc); + _channel->argv = (char**) calloc(channel->argc, sizeof(char*)); + if (!_channel->argv) + goto out_free; for (index = 0; index < _channel->argc; index++) { _channel->argv[index] = _strdup(channel->argv[index]); + if (!_channel->argv[index]) + goto out_release_args; } return _channel; + +out_release_args: + for (index = 0; _channel->argv[index]; index++) + free(_channel->argv[index]); +out_free: + free(_channel); + return NULL; } void freerdp_static_channel_collection_free(rdpSettings* settings) @@ -375,16 +451,29 @@ ADDIN_ARGV* freerdp_dynamic_channel_clone(ADDIN_ARGV* channel) ADDIN_ARGV* _channel = NULL; _channel = (ADDIN_ARGV*) malloc(sizeof(ADDIN_ARGV)); + if (!_channel) + return NULL; _channel->argc = channel->argc; _channel->argv = (char**) malloc(sizeof(char*) * channel->argc); + if (!_channel->argv) + goto out_free; for (index = 0; index < _channel->argc; index++) { _channel->argv[index] = _strdup(channel->argv[index]); + if (!_channel->argv[index]) + goto out_release_args; } return _channel; + +out_release_args: + for (index = 0; _channel->argv[index]; index++) + free(_channel->argv[index]); +out_free: + free(_channel); + return NULL; } void freerdp_dynamic_channel_collection_free(rdpSettings* settings) diff --git a/libfreerdp/core/bulk.c b/libfreerdp/core/bulk.c index 29319b1..1b12738 100644 --- a/libfreerdp/core/bulk.c +++ b/libfreerdp/core/bulk.c @@ -183,13 +183,13 @@ rdpBulk* bulk_new(rdpContext* context) void bulk_free(rdpBulk* bulk) { - if (bulk) - { - mppc_context_free(bulk->mppcSend); - mppc_context_free(bulk->mppcRecv); + if (!bulk) + return; - ncrush_context_free(bulk->ncrushRecv); + mppc_context_free(bulk->mppcSend); + mppc_context_free(bulk->mppcRecv); - free(bulk); - } + ncrush_context_free(bulk->ncrushRecv); + + free(bulk); } diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c index e679cfb..c910f15 100644 --- a/libfreerdp/core/certificate.c +++ b/libfreerdp/core/certificate.c @@ -161,6 +161,8 @@ BOOL certificate_read_x509_certificate(rdpCertBlob* cert, rdpCertInfo* info) int error = 0; s = Stream_New(cert->data, cert->length); + if (!s) + return FALSE; info->Modulus = 0; if (!ber_read_sequence_tag(s, &length)) /* Certificate (SEQUENCE) */ @@ -253,6 +255,8 @@ BOOL certificate_read_x509_certificate(rdpCertBlob* cert, rdpCertInfo* info) info->ModulusLength = modulus_length; info->Modulus = (BYTE*) malloc(info->ModulusLength); + if (!info->Modulus) + goto error1; Stream_Read(s, info->Modulus, info->ModulusLength); error++; @@ -290,12 +294,17 @@ rdpX509CertChain* certificate_new_x509_certificate_chain(UINT32 count) { rdpX509CertChain* x509_cert_chain; - x509_cert_chain = (rdpX509CertChain*) malloc(sizeof(rdpX509CertChain)); + x509_cert_chain = (rdpX509CertChain *)malloc(sizeof(rdpX509CertChain)); + if (!x509_cert_chain) + return NULL; x509_cert_chain->count = count; - x509_cert_chain->array = (rdpCertBlob*) malloc(sizeof(rdpCertBlob) * count); - ZeroMemory(x509_cert_chain->array, sizeof(rdpCertBlob) * count); - + x509_cert_chain->array = (rdpCertBlob *)calloc(count, sizeof(rdpCertBlob)); + if (!x509_cert_chain->array) + { + free(x509_cert_chain); + return NULL; + } return x509_cert_chain; } @@ -308,17 +317,17 @@ void certificate_free_x509_certificate_chain(rdpX509CertChain* x509_cert_chain) { int i; - if (x509_cert_chain != NULL) - { - for (i = 0; i < (int) x509_cert_chain->count; i++) - { - if (x509_cert_chain->array[i].data != NULL) - free(x509_cert_chain->array[i].data); - } + if (!x509_cert_chain) + return; - free(x509_cert_chain->array); - free(x509_cert_chain); + for (i = 0; i < (int)x509_cert_chain->count; i++) + { + if (x509_cert_chain->array[i].data) + free(x509_cert_chain->array[i].data); } + + free(x509_cert_chain->array); + free(x509_cert_chain); } static BOOL certificate_process_server_public_key(rdpCertificate* certificate, wStream* s, UINT32 length) @@ -335,7 +344,7 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w if (memcmp(magic, "RSA1", 4) != 0) { - fprintf(stderr, "gcc_process_server_public_key: magic error\n"); + fprintf(stderr, "%s: magic error\n", __FUNCTION__); return FALSE; } @@ -349,6 +358,8 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w return FALSE; certificate->cert_info.ModulusLength = modlen; 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); @@ -366,6 +377,8 @@ static BOOL certificate_process_server_public_signature(rdpCertificate* certific BYTE md5hash[CRYPTO_MD5_DIGEST_LENGTH]; md5ctx = crypto_md5_init(); + if (!md5ctx) + return FALSE; crypto_md5_update(md5ctx, sigdata, sigdatalen); crypto_md5_final(md5ctx, md5hash); @@ -378,18 +391,19 @@ static BOOL certificate_process_server_public_signature(rdpCertificate* certific if (sum != 0) { - fprintf(stderr, "certificate_process_server_public_signature: invalid signature\n"); + fprintf(stderr, "%s: invalid signature\n", __FUNCTION__); //return FALSE; } siglen -= 8; + // TODO: check the result of decrypt crypto_rsa_public_decrypt(encsig, siglen, TSSK_KEY_LENGTH, tssk_modulus, tssk_exponent, sig); /* Verify signature. */ if (memcmp(md5hash, sig, sizeof(md5hash)) != 0) { - fprintf(stderr, "certificate_process_server_public_signature: invalid signature\n"); + fprintf(stderr, "%s: invalid signature\n", __FUNCTION__); //return FALSE; } @@ -405,7 +419,7 @@ static BOOL certificate_process_server_public_signature(rdpCertificate* certific if (sig[16] != 0x00 || sum != 0xFF * (62 - 17) || sig[62] != 0x01) { - fprintf(stderr, "certificate_process_server_public_signature: invalid signature\n"); + fprintf(stderr, "%s: invalid signature\n", __FUNCTION__); //return FALSE; } @@ -439,7 +453,8 @@ BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate if (!(dwSigAlgId == SIGNATURE_ALG_RSA && dwKeyAlgId == KEY_EXCHANGE_ALG_RSA)) { - fprintf(stderr, "certificate_read_server_proprietary_certificate: parse error 1\n"); + fprintf(stderr, "%s: unsupported signature or key algorithm, dwSigAlgId=%d dwKeyAlgId=%d\n", + __FUNCTION__, dwSigAlgId, dwKeyAlgId); return FALSE; } @@ -447,7 +462,7 @@ BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate if (wPublicKeyBlobType != BB_RSA_KEY_BLOB) { - fprintf(stderr, "certificate_read_server_proprietary_certificate: parse error 2\n"); + fprintf(stderr, "%s: unsupported public key blob type %d\n", __FUNCTION__, wPublicKeyBlobType); return FALSE; } @@ -457,7 +472,7 @@ BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate if (!certificate_process_server_public_key(certificate, s, wPublicKeyBlobLen)) { - fprintf(stderr, "certificate_read_server_proprietary_certificate: parse error 3\n"); + fprintf(stderr, "%s: error in server public key\n", __FUNCTION__); return FALSE; } @@ -469,23 +484,26 @@ BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate if (wSignatureBlobType != BB_RSA_SIGNATURE_BLOB) { - fprintf(stderr, "certificate_read_server_proprietary_certificate: parse error 4\n"); + fprintf(stderr, "%s: unsupported blob signature %d\n", __FUNCTION__, wSignatureBlobType); return FALSE; } Stream_Read_UINT16(s, wSignatureBlobLen); if (Stream_GetRemainingLength(s) < wSignatureBlobLen) + { + fprintf(stderr, "%s: not enought bytes for signature(len=%d)\n", __FUNCTION__, wSignatureBlobLen); return FALSE; + } if (wSignatureBlobLen != 72) { - fprintf(stderr, "certificate_process_server_public_signature: invalid signature length (got %d, expected %d)\n", wSignatureBlobLen, 64); + fprintf(stderr, "%s: invalid signature length (got %d, expected %d)\n", __FUNCTION__, wSignatureBlobLen, 64); return FALSE; } if (!certificate_process_server_public_signature(certificate, sigdata, sigdatalen, s, wSignatureBlobLen)) { - fprintf(stderr, "certificate_read_server_proprietary_certificate: parse error 5\n"); + fprintf(stderr, "%s: unable to parse server public signature\n", __FUNCTION__); return FALSE; } @@ -512,6 +530,8 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, Stream_Read_UINT32(s, numCertBlobs); /* numCertBlobs */ certificate->x509_cert_chain = certificate_new_x509_certificate_chain(numCertBlobs); + if (!certificate->x509_cert_chain) + return FALSE; for (i = 0; i < (int) numCertBlobs; i++) { @@ -526,6 +546,8 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, DEBUG_CERTIFICATE("\nX.509 Certificate #%d, length:%d", i + 1, certLength); certificate->x509_cert_chain->array[i].data = (BYTE*) malloc(certLength); + if (!certificate->x509_cert_chain->array[i].data) + return FALSE; Stream_Read(s, certificate->x509_cert_chain->array[i].data, certLength); certificate->x509_cert_chain->array[i].length = certLength; @@ -608,29 +630,24 @@ rdpRsaKey* key_new(const char* keyfile) RSA* rsa; rdpRsaKey* key; - key = (rdpRsaKey*) malloc(sizeof(rdpRsaKey)); - ZeroMemory(key, sizeof(rdpRsaKey)); - - if (key == NULL) + key = (rdpRsaKey *)calloc(1, sizeof(rdpRsaKey)); + if (!key) return NULL; fp = fopen(keyfile, "r"); - if (fp == NULL) { - fprintf(stderr, "unable to load RSA key from %s: %s.", keyfile, strerror(errno)); - free(key) ; - return NULL; + fprintf(stderr, "%s: unable to open RSA key file %s: %s.", __FUNCTION__, keyfile, strerror(errno)); + goto out_free; } rsa = PEM_read_RSAPrivateKey(fp, NULL, NULL, NULL); - if (rsa == NULL) { - ERR_print_errors_fp(stdout); + fprintf(stderr, "%s: unable to load RSA key from %s: %s.", __FUNCTION__, keyfile, strerror(errno)); + ERR_print_errors_fp(stderr); fclose(fp); - free(key) ; - return NULL; + goto out_free; } fclose(fp); @@ -638,37 +655,36 @@ rdpRsaKey* key_new(const char* keyfile) switch (RSA_check_key(rsa)) { case 0: - RSA_free(rsa); - fprintf(stderr, "invalid RSA key in %s", keyfile); - free(key) ; - return NULL; + fprintf(stderr, "%s: invalid RSA key in %s\n", __FUNCTION__, keyfile); + goto out_free_rsa; case 1: /* Valid key. */ break; default: + fprintf(stderr, "%s: unexpected error when checking RSA key from %s: %s.", __FUNCTION__, keyfile, strerror(errno)); ERR_print_errors_fp(stderr); - RSA_free(rsa); - free(key) ; - return NULL; + goto out_free_rsa; } if (BN_num_bytes(rsa->e) > 4) { - RSA_free(rsa); - fprintf(stderr, "RSA public exponent too large in %s", keyfile); - free(key) ; - return NULL; + fprintf(stderr, "%s: RSA public exponent too large in %s\n", __FUNCTION__, keyfile); + goto out_free_rsa; } key->ModulusLength = BN_num_bytes(rsa->n); - key->Modulus = (BYTE*) malloc(key->ModulusLength); + key->Modulus = (BYTE *)malloc(key->ModulusLength); + if (!key->Modulus) + goto out_free_rsa; BN_bn2bin(rsa->n, key->Modulus); crypto_reverse(key->Modulus, key->ModulusLength); key->PrivateExponentLength = BN_num_bytes(rsa->d); - key->PrivateExponent = (BYTE*) malloc(key->PrivateExponentLength); + key->PrivateExponent = (BYTE *)malloc(key->PrivateExponentLength); + if (!key->PrivateExponent) + goto out_free_modulus; BN_bn2bin(rsa->d, key->PrivateExponent); crypto_reverse(key->PrivateExponent, key->PrivateExponentLength); @@ -677,18 +693,26 @@ rdpRsaKey* key_new(const char* keyfile) crypto_reverse(key->exponent, sizeof(key->exponent)); RSA_free(rsa); - return key; + +out_free_modulus: + free(key->Modulus); +out_free_rsa: + RSA_free(rsa); +out_free: + free(key); + return NULL; } void key_free(rdpRsaKey* key) { - if (key != NULL) - { + if (!key) + return; + + if (key->Modulus) free(key->Modulus); - free(key->PrivateExponent); - free(key); - } + free(key->PrivateExponent); + free(key); } /** @@ -701,13 +725,9 @@ rdpCertificate* certificate_new() { rdpCertificate* certificate; - certificate = (rdpCertificate*) malloc(sizeof(rdpCertificate)); - ZeroMemory(certificate, sizeof(rdpCertificate)); - - if (certificate != NULL) - { - certificate->x509_cert_chain = NULL; - } + certificate = (rdpCertificate*) calloc(1, sizeof(rdpCertificate)); + if (!certificate) + return NULL; return certificate; } @@ -719,13 +739,13 @@ rdpCertificate* certificate_new() void certificate_free(rdpCertificate* certificate) { - if (certificate != NULL) - { - certificate_free_x509_certificate_chain(certificate->x509_cert_chain); + if (!certificate) + return; - if (certificate->cert_info.Modulus != NULL) - free(certificate->cert_info.Modulus); + certificate_free_x509_certificate_chain(certificate->x509_cert_chain); - free(certificate); - } + if (certificate->cert_info.Modulus) + free(certificate->cert_info.Modulus); + + free(certificate); } diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index eac8777..62e7b30 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -178,6 +178,8 @@ BOOL rdp_client_connect(rdpRdp* rdp) } rdp->settingsCopy = freerdp_settings_clone(settings); + if (!rdp->settingsCopy) + return FALSE; nego_init(rdp->nego); nego_set_target(rdp->nego, settings->ServerHostname, settings->ServerPort); @@ -421,17 +423,13 @@ static BOOL rdp_client_establish_keys(rdpRdp* rdp) Stream_SealLength(s); if (transport_write(rdp->mcs->transport, s) < 0) - { return FALSE; - } Stream_Free(s, TRUE); /* now calculate encrypt / decrypt and update keys */ if (!security_establish_keys(rdp->settings->ClientRandom, rdp)) - { return FALSE; - } rdp->do_crypt = TRUE; @@ -441,15 +439,40 @@ static BOOL rdp_client_establish_keys(rdpRdp* rdp) if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { rdp->fips_encrypt = crypto_des3_encrypt_init(rdp->fips_encrypt_key, fips_ivec); + if (!rdp->fips_encrypt) + { + fprintf(stderr, "%s: unable to allocate des3 encrypt key\n", __FUNCTION__); + return FALSE; + } rdp->fips_decrypt = crypto_des3_decrypt_init(rdp->fips_decrypt_key, fips_ivec); + if (!rdp->fips_decrypt) + { + fprintf(stderr, "%s: unable to allocate des3 decrypt key\n", __FUNCTION__); + return FALSE; + } rdp->fips_hmac = crypto_hmac_new(); + if (!rdp->fips_hmac) + { + fprintf(stderr, "%s: unable to allocate fips hmac\n", __FUNCTION__); + return FALSE; + } return TRUE; } rdp->rc4_decrypt_key = crypto_rc4_init(rdp->decrypt_key, rdp->rc4_key_len); - rdp->rc4_encrypt_key = crypto_rc4_init(rdp->encrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_decrypt_key) + { + fprintf(stderr, "%s: unable to allocate rc4 decrypt key\n", __FUNCTION__); + return FALSE; + } + rdp->rc4_encrypt_key = crypto_rc4_init(rdp->encrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_encrypt_key) + { + fprintf(stderr, "%s: unable to allocate rc4 encrypt key\n", __FUNCTION__); + return FALSE; + } return TRUE; } @@ -470,7 +493,7 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) if (!rdp_read_header(rdp, s, &length, &channel_id)) { - fprintf(stderr, "rdp_server_establish_keys: invalid RDP header\n"); + fprintf(stderr, "%s: invalid RDP header\n", __FUNCTION__); return FALSE; } @@ -479,7 +502,7 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) if ((sec_flags & SEC_EXCHANGE_PKT) == 0) { - fprintf(stderr, "rdp_server_establish_keys: missing SEC_EXCHANGE_PKT in security header\n"); + fprintf(stderr, "%s: missing SEC_EXCHANGE_PKT in security header\n", __FUNCTION__); return FALSE; } @@ -488,21 +511,21 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) Stream_Read_UINT32(s, rand_len); - if (Stream_GetRemainingLength(s) < rand_len + 8) /* include 8 bytes of padding */ + /* rand_len already includes 8 bytes of padding */ + if (Stream_GetRemainingLength(s) < rand_len) return FALSE; key_len = rdp->settings->RdpServerRsaKey->ModulusLength; if (rand_len != key_len + 8) { - fprintf(stderr, "rdp_server_establish_keys: invalid encrypted client random length\n"); + fprintf(stderr, "%s: invalid encrypted client random length\n", __FUNCTION__); return FALSE; } ZeroMemory(crypt_client_random, sizeof(crypt_client_random)); Stream_Read(s, crypt_client_random, rand_len); - /* 8 zero bytes of padding */ - Stream_Seek(s, 8); + mod = rdp->settings->RdpServerRsaKey->Modulus; priv_exp = rdp->settings->RdpServerRsaKey->PrivateExponent; crypto_rsa_private_decrypt(crypt_client_random, rand_len - 8, key_len, mod, priv_exp, client_random); @@ -521,14 +544,41 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { rdp->fips_encrypt = crypto_des3_encrypt_init(rdp->fips_encrypt_key, fips_ivec); + if (!rdp->fips_encrypt) + { + fprintf(stderr, "%s: unable to allocate des3 encrypt key\n", __FUNCTION__); + return FALSE; + } + rdp->fips_decrypt = crypto_des3_decrypt_init(rdp->fips_decrypt_key, fips_ivec); + if (!rdp->fips_decrypt) + { + fprintf(stderr, "%s: unable to allocate des3 decrypt key\n", __FUNCTION__); + return FALSE; + } rdp->fips_hmac = crypto_hmac_new(); + if (!rdp->fips_hmac) + { + fprintf(stderr, "%s: unable to allocate fips hmac\n", __FUNCTION__); + return FALSE; + } return TRUE; } rdp->rc4_decrypt_key = crypto_rc4_init(rdp->decrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_decrypt_key) + { + fprintf(stderr, "%s: unable to allocate rc4 decrypt key\n", __FUNCTION__); + return FALSE; + } + rdp->rc4_encrypt_key = crypto_rc4_init(rdp->encrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_encrypt_key) + { + fprintf(stderr, "%s: unable to allocate rc4 encrypt key\n", __FUNCTION__); + return FALSE; + } return TRUE; } @@ -872,33 +922,35 @@ BOOL rdp_server_accept_nego(rdpRdp* rdp, wStream* s) { BOOL status; rdpSettings* settings = rdp->settings; + rdpNego *nego = rdp->nego; transport_set_blocking_mode(rdp->transport, TRUE); - if (!nego_read_request(rdp->nego, s)) + if (!nego_read_request(nego, s)) return FALSE; - rdp->nego->selected_protocol = 0; + nego->selected_protocol = 0; fprintf(stderr, "Client Security: NLA:%d TLS:%d RDP:%d\n", - (rdp->nego->requested_protocols & PROTOCOL_NLA) ? 1 : 0, - (rdp->nego->requested_protocols & PROTOCOL_TLS) ? 1 : 0, - (rdp->nego->requested_protocols == PROTOCOL_RDP) ? 1: 0); + (nego->requested_protocols & PROTOCOL_NLA) ? 1 : 0, + (nego->requested_protocols & PROTOCOL_TLS) ? 1 : 0, + (nego->requested_protocols == PROTOCOL_RDP) ? 1 : 0 + ); fprintf(stderr, "Server Security: NLA:%d TLS:%d RDP:%d\n", settings->NlaSecurity, settings->TlsSecurity, settings->RdpSecurity); - if ((settings->NlaSecurity) && (rdp->nego->requested_protocols & PROTOCOL_NLA)) + if ((settings->NlaSecurity) && (nego->requested_protocols & PROTOCOL_NLA)) { - rdp->nego->selected_protocol = PROTOCOL_NLA; + nego->selected_protocol = PROTOCOL_NLA; } - else if ((settings->TlsSecurity) && (rdp->nego->requested_protocols & PROTOCOL_TLS)) + else if ((settings->TlsSecurity) && (nego->requested_protocols & PROTOCOL_TLS)) { - rdp->nego->selected_protocol = PROTOCOL_TLS; + nego->selected_protocol = PROTOCOL_TLS; } - else if ((settings->RdpSecurity) && (rdp->nego->selected_protocol == PROTOCOL_RDP)) + else if ((settings->RdpSecurity) && (nego->selected_protocol == PROTOCOL_RDP)) { - rdp->nego->selected_protocol = PROTOCOL_RDP; + nego->selected_protocol = PROTOCOL_RDP; } else { @@ -906,20 +958,21 @@ BOOL rdp_server_accept_nego(rdpRdp* rdp, wStream* s) } fprintf(stderr, "Negotiated Security: NLA:%d TLS:%d RDP:%d\n", - (rdp->nego->selected_protocol & PROTOCOL_NLA) ? 1 : 0, - (rdp->nego->selected_protocol & PROTOCOL_TLS) ? 1 : 0, - (rdp->nego->selected_protocol == PROTOCOL_RDP) ? 1: 0); + (nego->selected_protocol & PROTOCOL_NLA) ? 1 : 0, + (nego->selected_protocol & PROTOCOL_TLS) ? 1 : 0, + (nego->selected_protocol == PROTOCOL_RDP) ? 1: 0 + ); - if (!nego_send_negotiation_response(rdp->nego)) + if (!nego_send_negotiation_response(nego)) return FALSE; status = FALSE; - if (rdp->nego->selected_protocol & PROTOCOL_NLA) + if (nego->selected_protocol & PROTOCOL_NLA) status = transport_accept_nla(rdp->transport); - else if (rdp->nego->selected_protocol & PROTOCOL_TLS) + else if (nego->selected_protocol & PROTOCOL_TLS) status = transport_accept_tls(rdp->transport); - else if (rdp->nego->selected_protocol == PROTOCOL_RDP) /* 0 */ + else if (nego->selected_protocol == PROTOCOL_RDP) /* 0 */ status = transport_accept_rdp(rdp->transport); if (!status) diff --git a/libfreerdp/core/gcc.c b/libfreerdp/core/gcc.c index c7cb18f..9e716b0 100644 --- a/libfreerdp/core/gcc.c +++ b/libfreerdp/core/gcc.c @@ -1167,11 +1167,17 @@ void gcc_write_server_security_data(wStream* s, rdpMcs* mcs) sigDataLen = Stream_Pointer(s) - sigData; Stream_Write_UINT16(s, BB_RSA_SIGNATURE_BLOB); /* wSignatureBlobType */ - Stream_Write_UINT16(s, keyLen + 8); /* wSignatureBlobLen */ + Stream_Write_UINT16(s, sizeof(encryptedSignature) + 8); /* wSignatureBlobLen */ memcpy(signature, initial_signature, sizeof(initial_signature)); md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } + crypto_md5_update(md5, sigData, sigDataLen); crypto_md5_final(md5, signature); diff --git a/libfreerdp/core/info.c b/libfreerdp/core/info.c index ad76e23..6e16968 100644 --- a/libfreerdp/core/info.c +++ b/libfreerdp/core/info.c @@ -248,6 +248,11 @@ void rdp_write_extended_info_packet(wStream* s, rdpSettings* settings) clientCookie->logonId = serverCookie->logonId; hmac = crypto_hmac_new(); + if (!hmac) + { + fprintf(stderr, "%s: unable to allocate hmac\n", __FUNCTION__); + goto out_free; + } crypto_hmac_md5_init(hmac, serverCookie->arcRandomBits, 16); @@ -270,11 +275,12 @@ void rdp_write_extended_info_packet(wStream* s, rdpSettings* settings) rdp_write_client_auto_reconnect_cookie(s, settings); /* autoReconnectCookie */ /* mark as used */ settings->ServerAutoReconnectCookie->cbLen = 0; + crypto_hmac_free(hmac); } /* reserved1 (2 bytes) */ /* reserved2 (2 bytes) */ - +out_free: free(clientAddress); free(clientDir); } diff --git a/libfreerdp/core/license.c b/libfreerdp/core/license.c index 78193f3..e91aba6 100644 --- a/libfreerdp/core/license.c +++ b/libfreerdp/core/license.c @@ -392,6 +392,12 @@ void license_generate_hwid(rdpLicense* license) mac_address = license->rdp->transport->TcpIn->mac_address; md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } + crypto_md5_update(md5, mac_address, 6); crypto_md5_final(md5, &license->HardwareId[HWID_PLATFORM_ID_LENGTH]); } @@ -458,6 +464,11 @@ void license_decrypt_platform_challenge(rdpLicense* license) license->PlatformChallenge->length = license->EncryptedPlatformChallenge->length; rc4 = crypto_rc4_init(license->LicensingEncryptionKey, LICENSING_ENCRYPTION_KEY_LENGTH); + if (!rc4) + { + fprintf(stderr, "%s: unable to allocate a rc4\n", __FUNCTION__); + return; + } crypto_rc4(rc4, license->EncryptedPlatformChallenge->length, license->EncryptedPlatformChallenge->data, @@ -1042,6 +1053,11 @@ void license_send_platform_challenge_response_packet(rdpLicense* license) buffer = (BYTE*) malloc(HWID_LENGTH); rc4 = crypto_rc4_init(license->LicensingEncryptionKey, LICENSING_ENCRYPTION_KEY_LENGTH); + if (!rc4) + { + fprintf(stderr, "%s: unable to allocate a rc4\n", __FUNCTION__); + return; + } crypto_rc4(rc4, HWID_LENGTH, license->HardwareId, buffer); crypto_rc4_free(rc4); diff --git a/libfreerdp/core/nego.c b/libfreerdp/core/nego.c index ef1053f..c53c6e6 100644 --- a/libfreerdp/core/nego.c +++ b/libfreerdp/core/nego.c @@ -497,9 +497,10 @@ BOOL nego_recv_response(rdpNego* nego) wStream* s; s = Stream_New(NULL, 1024); + if (!s) + return FALSE; status = transport_read(nego->transport, s); - if (status < 0) { Stream_Free(s, TRUE); @@ -869,6 +870,8 @@ BOOL nego_send_negotiation_response(rdpNego* nego) settings = nego->transport->settings; s = Stream_New(NULL, 512); + if (!s) + return FALSE; length = TPDU_CONNECTION_CONFIRM_LENGTH; bm = Stream_GetPosition(s); @@ -899,7 +902,7 @@ BOOL nego_send_negotiation_response(rdpNego* nego) * TODO: Check for other possibilities, * like SSL_NOT_ALLOWED_BY_SERVER. */ - fprintf(stderr, "nego_send_negotiation_response: client supports only Standard RDP Security\n"); + fprintf(stderr, "%s: client supports only Standard RDP Security\n", __FUNCTION__); Stream_Write_UINT32(s, SSL_REQUIRED_BY_SERVER); length += 8; status = FALSE; @@ -940,7 +943,7 @@ BOOL nego_send_negotiation_response(rdpNego* nego) settings->EncryptionLevel = ENCRYPTION_LEVEL_CLIENT_COMPATIBLE; } - if (settings->DisableEncryption && settings->RdpServerRsaKey == NULL && settings->RdpKeyFile == NULL) + if (settings->DisableEncryption && !settings->RdpServerRsaKey && !settings->RdpKeyFile) return FALSE; } else if (settings->SelectedProtocol == PROTOCOL_TLS) @@ -990,15 +993,13 @@ void nego_init(rdpNego* nego) rdpNego* nego_new(rdpTransport* transport) { - rdpNego* nego = (rdpNego*) malloc(sizeof(rdpNego)); + rdpNego* nego = (rdpNego*) calloc(1, sizeof(rdpNego)); + if (!nego) + return NULL; - if (nego) - { - ZeroMemory(nego, sizeof(rdpNego)); - nego->transport = transport; - nego_init(nego); - } + nego->transport = transport; + nego_init(nego); return nego; } diff --git a/libfreerdp/core/peer.c b/libfreerdp/core/peer.c index 24bc3cf..191739f 100644 --- a/libfreerdp/core/peer.c +++ b/libfreerdp/core/peer.c @@ -36,15 +36,22 @@ extern const char* DATA_PDU_TYPE_STRINGS[80]; static BOOL freerdp_peer_initialize(freerdp_peer* client) { - client->context->rdp->settings->ServerMode = TRUE; - client->context->rdp->settings->FrameAcknowledge = 0; - client->context->rdp->settings->LocalConnection = client->local; - client->context->rdp->state = CONNECTION_STATE_INITIAL; + rdpRdp *rdp = client->context->rdp; + rdpSettings *settings = rdp->settings; - if (client->context->rdp->settings->RdpKeyFile != NULL) + settings->ServerMode = TRUE; + settings->FrameAcknowledge = 0; + settings->LocalConnection = client->local; + rdp->state = CONNECTION_STATE_INITIAL; + + if (settings->RdpKeyFile != NULL) { - client->context->rdp->settings->RdpServerRsaKey = - key_new(client->context->rdp->settings->RdpKeyFile); + settings->RdpServerRsaKey = key_new(settings->RdpKeyFile); + if (!settings->RdpServerRsaKey) + { + fprintf(stderr, "%s: inavlid RDP key file %s\n", __FUNCTION__, settings->RdpKeyFile); + return FALSE; + } } return TRUE; @@ -304,8 +311,8 @@ static int peer_recv_callback(rdpTransport* transport, wStream* s, void* extra) } rdp_server_transition_to_state(rdp, CONNECTION_STATE_SECURE_SETTINGS_EXCHANGE); - return peer_recv_callback(transport, s, extra); - + if (Stream_GetRemainingLength(s) > 0) + return peer_recv_callback(transport, s, extra); break; case CONNECTION_STATE_SECURE_SETTINGS_EXCHANGE: diff --git a/libfreerdp/core/security.c b/libfreerdp/core/security.c index 5684528..0712788 100644 --- a/libfreerdp/core/security.c +++ b/libfreerdp/core/security.c @@ -129,6 +129,11 @@ static void security_salted_hash(const BYTE* salt, const BYTE* input, int length /* SHA1_Digest = SHA1(Input + Salt + Salt1 + Salt2) */ sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return; + } crypto_sha1_update(sha1, input, length); /* Input */ crypto_sha1_update(sha1, salt, 48); /* Salt (48 bytes) */ crypto_sha1_update(sha1, salt1, 32); /* Salt1 (32 bytes) */ @@ -137,6 +142,11 @@ static void security_salted_hash(const BYTE* salt, const BYTE* input, int length /* SaltedHash(Salt, Input, Salt1, Salt2) = MD5(S + SHA1_Digest) */ md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } crypto_md5_update(md5, salt, 48); /* Salt (48 bytes) */ crypto_md5_update(md5, sha1_digest, sizeof(sha1_digest)); /* SHA1_Digest */ crypto_md5_final(md5, output); @@ -185,6 +195,11 @@ void security_md5_16_32_32(const BYTE* in0, const BYTE* in1, const BYTE* in2, BY CryptoMd5 md5; md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } crypto_md5_update(md5, in0, 16); crypto_md5_update(md5, in1, 32); crypto_md5_update(md5, in2, 32); @@ -220,6 +235,11 @@ void security_mac_data(const BYTE* mac_salt_key, const BYTE* data, UINT32 length /* SHA1_Digest = SHA1(MacSaltKey + pad1 + length + data) */ sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return; + } crypto_sha1_update(sha1, mac_salt_key, 16); /* MacSaltKey */ crypto_sha1_update(sha1, pad1, sizeof(pad1)); /* pad1 */ crypto_sha1_update(sha1, length_le, sizeof(length_le)); /* length */ @@ -228,6 +248,11 @@ void security_mac_data(const BYTE* mac_salt_key, const BYTE* data, UINT32 length /* MacData = MD5(MacSaltKey + pad2 + SHA1_Digest) */ md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } crypto_md5_update(md5, mac_salt_key, 16); /* MacSaltKey */ crypto_md5_update(md5, pad2, sizeof(pad2)); /* pad2 */ crypto_md5_update(md5, sha1_digest, sizeof(sha1_digest)); /* SHA1_Digest */ @@ -246,6 +271,11 @@ void security_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, BYTE* /* SHA1_Digest = SHA1(MACKeyN + pad1 + length + data) */ sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return; + } crypto_sha1_update(sha1, rdp->sign_key, rdp->rc4_key_len); /* MacKeyN */ crypto_sha1_update(sha1, pad1, sizeof(pad1)); /* pad1 */ crypto_sha1_update(sha1, length_le, sizeof(length_le)); /* length */ @@ -254,6 +284,11 @@ void security_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, BYTE* /* MACSignature = First64Bits(MD5(MACKeyN + pad2 + SHA1_Digest)) */ md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } crypto_md5_update(md5, rdp->sign_key, rdp->rc4_key_len); /* MacKeyN */ crypto_md5_update(md5, pad2, sizeof(pad2)); /* pad2 */ crypto_md5_update(md5, sha1_digest, sizeof(sha1_digest)); /* SHA1_Digest */ @@ -289,6 +324,11 @@ void security_salted_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, /* SHA1_Digest = SHA1(MACKeyN + pad1 + length + data) */ sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return; + } crypto_sha1_update(sha1, rdp->sign_key, rdp->rc4_key_len); /* MacKeyN */ crypto_sha1_update(sha1, pad1, sizeof(pad1)); /* pad1 */ crypto_sha1_update(sha1, length_le, sizeof(length_le)); /* length */ @@ -298,6 +338,11 @@ void security_salted_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, /* MACSignature = First64Bits(MD5(MACKeyN + pad2 + SHA1_Digest)) */ md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return; + } crypto_md5_update(md5, rdp->sign_key, rdp->rc4_key_len); /* MacKeyN */ crypto_md5_update(md5, pad2, sizeof(pad2)); /* pad2 */ crypto_md5_update(md5, sha1_digest, sizeof(sha1_digest)); /* SHA1_Digest */ @@ -379,6 +424,11 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) rdp->settings->FastPathInput = FALSE; sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return FALSE; + } crypto_sha1_update(sha1, client_random + 16, 16); crypto_sha1_update(sha1, server_random + 16, 16); crypto_sha1_final(sha1, client_encrypt_key_t); @@ -387,6 +437,11 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) fips_expand_key_bits(client_encrypt_key_t, rdp->fips_encrypt_key); sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return FALSE; + } crypto_sha1_update(sha1, client_random, 16); crypto_sha1_update(sha1, server_random, 16); crypto_sha1_final(sha1, client_decrypt_key_t); @@ -395,6 +450,11 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) fips_expand_key_bits(client_decrypt_key_t, rdp->fips_decrypt_key); sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return FALSE; + } crypto_sha1_update(sha1, client_decrypt_key_t, 20); crypto_sha1_update(sha1, client_encrypt_key_t, 20); crypto_sha1_final(sha1, rdp->fips_sign_key); @@ -454,18 +514,33 @@ BOOL security_key_update(BYTE* key, BYTE* update_key, int key_len) BYTE salt40[] = { 0xD1, 0x26, 0x9E }; sha1 = crypto_sha1_init(); + if (!sha1) + { + fprintf(stderr, "%s: unable to allocate a sha1\n", __FUNCTION__); + return FALSE; + } crypto_sha1_update(sha1, update_key, key_len); crypto_sha1_update(sha1, pad1, sizeof(pad1)); crypto_sha1_update(sha1, key, key_len); crypto_sha1_final(sha1, sha1h); md5 = crypto_md5_init(); + if (!md5) + { + fprintf(stderr, "%s: unable to allocate a md5\n", __FUNCTION__); + return FALSE; + } crypto_md5_update(md5, update_key, key_len); crypto_md5_update(md5, pad2, sizeof(pad2)); crypto_md5_update(md5, sha1h, sizeof(sha1h)); crypto_md5_final(md5, key); rc4 = crypto_rc4_init(key, key_len); + if (!rc4) + { + fprintf(stderr, "%s: unable to allocate a rc4\n", __FUNCTION__); + return FALSE; + } crypto_rc4(rc4, key_len, key, key); crypto_rc4_free(rc4); @@ -482,6 +557,11 @@ BOOL security_encrypt(BYTE* data, int length, rdpRdp* rdp) security_key_update(rdp->encrypt_key, rdp->encrypt_update_key, rdp->rc4_key_len); crypto_rc4_free(rdp->rc4_encrypt_key); rdp->rc4_encrypt_key = crypto_rc4_init(rdp->encrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_encrypt_key) + { + fprintf(stderr, "%s: unable to allocate rc4 encrypt key\n", __FUNCTION__); + return FALSE; + } rdp->encrypt_use_count = 0; } crypto_rc4(rdp->rc4_encrypt_key, length, data, data); @@ -499,6 +579,12 @@ BOOL security_decrypt(BYTE* data, int length, rdpRdp* rdp) security_key_update(rdp->decrypt_key, rdp->decrypt_update_key, rdp->rc4_key_len); crypto_rc4_free(rdp->rc4_decrypt_key); rdp->rc4_decrypt_key = crypto_rc4_init(rdp->decrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_decrypt_key) + { + fprintf(stderr, "%s: unable to allocate rc4 decrypt key\n", __FUNCTION__); + return FALSE; + } + rdp->decrypt_use_count = 0; } crypto_rc4(rdp->rc4_decrypt_key, length, data, data); diff --git a/libfreerdp/crypto/certificate.c b/libfreerdp/crypto/certificate.c index 3d86944..1cfe0f6 100644 --- a/libfreerdp/crypto/certificate.c +++ b/libfreerdp/crypto/certificate.c @@ -231,17 +231,23 @@ rdpCertificateData* certificate_data_new(char* hostname, char* fingerprint) { rdpCertificateData* certdata; - certdata = (rdpCertificateData*) malloc(sizeof(rdpCertificateData)); - - if (certdata != NULL) - { - ZeroMemory(certdata, sizeof(rdpCertificateData)); - - certdata->hostname = _strdup(hostname); - certdata->fingerprint = _strdup(fingerprint); - } - + certdata = (rdpCertificateData *)calloc(1, sizeof(rdpCertificateData)); + if (!certdata) + return NULL; + + certdata->hostname = _strdup(hostname); + if (!certdata->hostname) + goto out_free; + certdata->fingerprint = _strdup(fingerprint); + if (!certdata->fingerprint) + goto out_free_hostname; return certdata; + +out_free_hostname: + free(certdata->hostname); +out_free: + free(certdata); + return NULL; } void certificate_data_free(rdpCertificateData* certificate_data) diff --git a/libfreerdp/crypto/crypto.c b/libfreerdp/crypto/crypto.c index 2445187..b3d3fbb 100644 --- a/libfreerdp/crypto/crypto.c +++ b/libfreerdp/crypto/crypto.c @@ -28,6 +28,8 @@ CryptoSha1 crypto_sha1_init(void) { CryptoSha1 sha1 = malloc(sizeof(*sha1)); + if (!sha1) + return NULL; SHA1_Init(&sha1->sha_ctx); return sha1; } @@ -46,6 +48,8 @@ void crypto_sha1_final(CryptoSha1 sha1, BYTE* out_data) CryptoMd5 crypto_md5_init(void) { CryptoMd5 md5 = malloc(sizeof(*md5)); + if (!md5) + return NULL; MD5_Init(&md5->md5_ctx); return md5; } @@ -64,6 +68,8 @@ void crypto_md5_final(CryptoMd5 md5, BYTE* out_data) CryptoRc4 crypto_rc4_init(const BYTE* key, UINT32 length) { CryptoRc4 rc4 = malloc(sizeof(*rc4)); + if (!rc4) + return NULL; RC4_set_key(&rc4->rc4_key, length, key); return rc4; } @@ -82,6 +88,9 @@ void crypto_rc4_free(CryptoRc4 rc4) CryptoDes3 crypto_des3_encrypt_init(const BYTE* key, const BYTE* ivec) { CryptoDes3 des3 = malloc(sizeof(*des3)); + if (!des3) + return NULL; + EVP_CIPHER_CTX_init(&des3->des3_ctx); EVP_EncryptInit_ex(&des3->des3_ctx, EVP_des_ede3_cbc(), NULL, key, ivec); EVP_CIPHER_CTX_set_padding(&des3->des3_ctx, 0); @@ -91,6 +100,9 @@ CryptoDes3 crypto_des3_encrypt_init(const BYTE* key, const BYTE* ivec) CryptoDes3 crypto_des3_decrypt_init(const BYTE* key, const BYTE* ivec) { CryptoDes3 des3 = malloc(sizeof(*des3)); + if (!des3) + return NULL; + EVP_CIPHER_CTX_init(&des3->des3_ctx); EVP_DecryptInit_ex(&des3->des3_ctx, EVP_des_ede3_cbc(), NULL, key, ivec); EVP_CIPHER_CTX_set_padding(&des3->des3_ctx, 0); @@ -123,6 +135,9 @@ void crypto_des3_free(CryptoDes3 des3) CryptoHmac crypto_hmac_new(void) { CryptoHmac hmac = malloc(sizeof(*hmac)); + if (!hmac) + return NULL; + HMAC_CTX_init(&hmac->hmac_ctx); return hmac; } @@ -159,6 +174,9 @@ void crypto_hmac_free(CryptoHmac hmac) CryptoCert crypto_cert_read(BYTE* data, UINT32 length) { CryptoCert cert = malloc(sizeof(*cert)); + if (!cert) + return NULL; + /* this will move the data pointer but we don't care, we don't use it again */ cert->px509 = d2i_X509(NULL, (D2I_X509_CONST BYTE **) &data, length); return cert; @@ -182,19 +200,17 @@ BOOL crypto_cert_get_public_key(CryptoCert cert, BYTE** PublicKey, DWORD* Public EVP_PKEY* pkey = NULL; pkey = X509_get_pubkey(cert->px509); - if (!pkey) { - fprintf(stderr, "crypto_cert_get_public_key: X509_get_pubkey() failed\n"); + fprintf(stderr, "%s: X509_get_pubkey() failed\n", __FUNCTION__); status = FALSE; goto exit; } length = i2d_PublicKey(pkey, NULL); - if (length < 1) { - fprintf(stderr, "crypto_cert_get_public_key: i2d_PublicKey() failed\n"); + fprintf(stderr, "%s: i2d_PublicKey() failed\n", __FUNCTION__); status = FALSE; goto exit; } @@ -215,13 +231,15 @@ exit: static int crypto_rsa_common(const BYTE* input, int length, UINT32 key_length, const BYTE* modulus, const BYTE* exponent, int exponent_size, BYTE* output) { BN_CTX* ctx; - int output_length; + int output_length = -1; BYTE* input_reverse; BYTE* modulus_reverse; BYTE* exponent_reverse; BIGNUM mod, exp, x, y; input_reverse = (BYTE*) malloc(2 * key_length + exponent_size); + if (!input_reverse) + return -1; modulus_reverse = input_reverse + key_length; exponent_reverse = modulus_reverse + key_length; @@ -233,6 +251,8 @@ static int crypto_rsa_common(const BYTE* input, int length, UINT32 key_length, c crypto_reverse(input_reverse, length); ctx = BN_CTX_new(); + if (!ctx) + goto out_free_input_reverse; BN_init(&mod); BN_init(&exp); BN_init(&x); @@ -254,6 +274,8 @@ static int crypto_rsa_common(const BYTE* input, int length, UINT32 key_length, c BN_free(&exp); BN_free(&mod); BN_CTX_free(ctx); + +out_free_input_reverse: free(input_reverse); return output_length; @@ -322,11 +344,11 @@ char* crypto_cert_fingerprint(X509* xcert) X509_digest(xcert, EVP_sha1(), fp, &fp_len); - fp_buffer = (char*) malloc(3 * fp_len); - ZeroMemory(fp_buffer, 3 * fp_len); + fp_buffer = (char*) calloc(3, fp_len); + if (!fp_buffer) + return NULL; p = fp_buffer; - for (i = 0; i < (int) (fp_len - 1); i++) { sprintf(p, "%02x:", fp[i]); @@ -527,6 +549,9 @@ rdpCertificateData* crypto_get_certificate_data(X509* xcert, char* hostname) rdpCertificateData* certdata; fp = crypto_cert_fingerprint(xcert); + if (!fp) + return NULL; + certdata = certificate_data_new(hostname, fp); free(fp); @@ -542,6 +567,11 @@ void crypto_cert_print_info(X509* xcert) subject = crypto_cert_subject(xcert); issuer = crypto_cert_issuer(xcert); fp = crypto_cert_fingerprint(xcert); + if (!fp) + { + fprintf(stderr, "%s: error computing fingerprint\n", __FUNCTION__); + goto out_free_issuer; + } fprintf(stderr, "Certificate details:\n"); fprintf(stderr, "\tSubject: %s\n", subject); @@ -551,7 +581,8 @@ void crypto_cert_print_info(X509* xcert) "the CA certificate in your certificate store, or the certificate has expired. " "Please look at the documentation on how to create local certificate store for a private CA.\n"); - free(subject); - free(issuer); free(fp); +out_free_issuer: + free(issuer); + free(subject); } diff --git a/winpr/include/winpr/file.h b/winpr/include/winpr/file.h index 69e4ece..a264f4f 100644 --- a/winpr/include/winpr/file.h +++ b/winpr/include/winpr/file.h @@ -332,7 +332,7 @@ WINPR_API LPSTR FilePatternFindNextWildcardA(LPCSTR lpPattern, DWORD* pFlags); WINPR_API int UnixChangeFileMode(const char* filename, int flags); WINPR_API char* GetNamedPipeNameWithoutPrefixA(LPCSTR lpName); -WINPR_API char* GetNamedPipeUnixDomainSocketBaseFilePathA(); +WINPR_API char* GetNamedPipeUnixDomainSocketBaseFilePathA(void); WINPR_API char* GetNamedPipeUnixDomainSocketFilePathA(LPCSTR lpName); #ifdef __cplusplus -- 2.7.4