From 31570e31e1859c3d7b32fde8659dfbe49c4b283e Mon Sep 17 00:00:00 2001 From: akallabeth Date: Wed, 25 Sep 2024 05:35:51 +0200 Subject: [PATCH] [crypto] fix integer narrowing --- libfreerdp/crypto/crypto.c | 6 ++-- libfreerdp/crypto/privatekey.c | 11 +++++-- libfreerdp/crypto/tls.c | 35 +++++++++++----------- libfreerdp/crypto/x509_utils.c | 53 ++++++++++++++++------------------ 4 files changed, 54 insertions(+), 51 deletions(-) diff --git a/libfreerdp/crypto/crypto.c b/libfreerdp/crypto/crypto.c index adbbd6cd2..806f9b986 100644 --- a/libfreerdp/crypto/crypto.c +++ b/libfreerdp/crypto/crypto.c @@ -94,12 +94,12 @@ static SSIZE_T crypto_rsa_common(const BYTE* input, size_t length, UINT32 key_le if (!(y = BN_new())) goto fail; - if (!BN_bin2bn(modulus_reverse, key_length, mod)) + if (!BN_bin2bn(modulus_reverse, (int)key_length, mod)) goto fail; - if (!BN_bin2bn(exponent_reverse, exponent_size, exp)) + if (!BN_bin2bn(exponent_reverse, (int)exponent_size, exp)) goto fail; - if (!BN_bin2bn(input_reverse, length, x)) + if (!BN_bin2bn(input_reverse, (int)length, x)) goto fail; if (BN_mod_exp(y, x, exp, mod, ctx) != 1) goto fail; diff --git a/libfreerdp/crypto/privatekey.c b/libfreerdp/crypto/privatekey.c index 9a6fb25fd..09d1593ac 100644 --- a/libfreerdp/crypto/privatekey.c +++ b/libfreerdp/crypto/privatekey.c @@ -125,7 +125,11 @@ static EVP_PKEY* evp_pkey_utils_from_pem(const char* data, size_t len, BOOL from if (fromFile) bio = BIO_new_file(data, "rb"); else - bio = BIO_new_mem_buf(data, len); + { + if (len > INT_MAX) + return NULL; + bio = BIO_new_mem_buf(data, (int)len); + } if (!bio) { @@ -426,7 +430,10 @@ BOOL freerdp_key_generate(rdpPrivateKey* key, size_t key_length) if (EVP_PKEY_keygen_init(pctx) != 1) goto fail; - if (EVP_PKEY_CTX_set_rsa_keygen_bits(pctx, key_length) != 1) + if (key_length > INT_MAX) + goto fail; + + if (EVP_PKEY_CTX_set_rsa_keygen_bits(pctx, (int)key_length) != 1) goto fail; EVP_PKEY_free(key->evp); diff --git a/libfreerdp/crypto/tls.c b/libfreerdp/crypto/tls.c index ef3182882..4047cc525 100644 --- a/libfreerdp/crypto/tls.c +++ b/libfreerdp/crypto/tls.c @@ -240,16 +240,14 @@ static int bio_rdp_tls_read(BIO* bio, char* buf, int size) static int bio_rdp_tls_puts(BIO* bio, const char* str) { - size_t size = 0; - int status = 0; - if (!str) return 0; - size = strlen(str); + const size_t size = strnlen(str, INT_MAX + 1UL); + if (size > INT_MAX) + return -1; ERR_clear_error(); - status = BIO_write(bio, str, size); - return status; + return BIO_write(bio, str, (int)size); } static int bio_rdp_tls_gets(BIO* bio, char* str, int size) @@ -262,7 +260,7 @@ static long bio_rdp_tls_ctrl(BIO* bio, int cmd, long num, void* ptr) BIO* ssl_rbio = NULL; BIO* ssl_wbio = NULL; BIO* next_bio = NULL; - int status = -1; + long status = -1; BIO_RDP_TLS* tls = (BIO_RDP_TLS*)BIO_get_data(bio); if (!tls) @@ -1087,7 +1085,7 @@ TlsHandshakeResult freerdp_tls_accept_ex(rdpTls* tls, BIO* underlying, rdpSettin { WINPR_ASSERT(tls); - long options = 0; + int options = 0; int status = 0; /** @@ -1239,33 +1237,34 @@ BOOL freerdp_tls_send_alert(rdpTls* tls) int freerdp_tls_write_all(rdpTls* tls, const BYTE* data, int length) { WINPR_ASSERT(tls); - int status = 0; int offset = 0; BIO* bio = tls->bio; while (offset < length) { ERR_clear_error(); - status = BIO_write(bio, &data[offset], length - offset); + const int rc = BIO_write(bio, &data[offset], length - offset); - if (status > 0) - { - offset += status; - } + if (rc < 0) + return rc; + + if (rc > 0) + offset += rc; else { if (!BIO_should_retry(bio)) return -1; if (BIO_write_blocked(bio)) - status = BIO_wait_write(bio, 100); + { + const long status = BIO_wait_write(bio, 100); + if (status < 0) + return -1; + } else if (BIO_read_blocked(bio)) return -2; /* Abort write, there is data that must be read */ else USleep(100); - - if (status < 0) - return -1; } } diff --git a/libfreerdp/crypto/x509_utils.c b/libfreerdp/crypto/x509_utils.c index f362e06bb..d4226b72e 100644 --- a/libfreerdp/crypto/x509_utils.c +++ b/libfreerdp/crypto/x509_utils.c @@ -616,11 +616,7 @@ out_free_issuer: static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, size_t* plength) { - BIO* bio = NULL; - int status = 0; int count = 0; - size_t offset = 0; - size_t length = 0; BOOL rc = FALSE; BYTE* pemCert = NULL; @@ -631,7 +627,7 @@ static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, * Don't manage certificates internally, leave it up entirely to the external client * implementation */ - bio = BIO_new(BIO_s_mem()); + BIO* bio = BIO_new(BIO_s_mem()); if (!bio) { @@ -640,7 +636,7 @@ static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, } X509* wcert = WINPR_CAST_CONST_PTR_AWAY(xcert, X509*); - status = PEM_write_bio_X509(bio, wcert); + int status = PEM_write_bio_X509(bio, wcert); if (status < 0) { @@ -663,46 +659,42 @@ static BYTE* x509_utils_get_pem(const X509* xcert, const STACK_OF(X509) * chain, } } - offset = 0; - length = 2048; + const size_t blocksize = 2048; + size_t offset = 0; + size_t length = blocksize; pemCert = (BYTE*)malloc(length + 1); - if (!pemCert) + if (!pemCert || (length > INT_MAX)) { WLog_ERR(TAG, "error allocating pemCert"); goto fail; } - ERR_clear_error(); - status = BIO_read(bio, pemCert, length); - - if (status < 0) + while (offset < length) { - WLog_ERR(TAG, "failed to read certificate"); - goto fail; - } - - offset += (size_t)status; - - while (offset >= length) - { - int new_len = 0; - BYTE* new_cert = NULL; - new_len = length * 2; - new_cert = (BYTE*)realloc(pemCert, new_len + 1); + size_t new_len = length + blocksize; + BYTE* new_cert = (BYTE*)realloc(pemCert, new_len + 1); + size_t diff = length - offset; if (!new_cert) goto fail; length = new_len; pemCert = new_cert; + ERR_clear_error(); - status = BIO_read(bio, &pemCert[offset], length - offset); + + if (diff > INT_MAX) + goto fail; + + status = BIO_read(bio, &pemCert[offset], (int)diff); if (status < 0) break; - offset += status; + offset += (size_t)status; + if ((size_t)status < diff) + break; } if (status < 0) @@ -735,7 +727,12 @@ X509* x509_utils_from_pem(const char* data, size_t len, BOOL fromFile) if (fromFile) bio = BIO_new_file(data, "rb"); else - bio = BIO_new_mem_buf(data, len); + { + if (len > INT_MAX) + return NULL; + + bio = BIO_new_mem_buf(data, (int)len); + } if (!bio) {