From 305a5ec49632399194b9760803af4587f8e7ef05 Mon Sep 17 00:00:00 2001 From: Piotr Nowicki Date: Mon, 10 Aug 2020 17:42:18 +0200 Subject: [PATCH] Checking in critical places if secured memset() and memcpy() was successful Signed-off-by: Piotr Nowicki --- library/entropy.c | 7 +++--- library/pkparse.c | 14 ++++++++--- library/rsa.c | 23 +++++++++++------ library/ssl_tls.c | 63 ++++++++++++++++++++++++++++++++++++++-------- tinycrypt/ecc_dh.c | 5 +++- 5 files changed, 87 insertions(+), 25 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index 8db3d94f1..2b1e7ef3a 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -462,9 +462,10 @@ int mbedtls_entropy_func( void *data, unsigned char *output, size_t len ) for( i = 0; i < ctx->source_count; i++ ) ctx->source[i].size = 0; - mbedtls_platform_memcpy( output, buf, len ); - - ret = 0; + if( output == mbedtls_platform_memcpy( output, buf, len ) ) + { + ret = 0; + } exit: mbedtls_platform_zeroize( buf, sizeof( buf ) ); diff --git a/library/pkparse.c b/library/pkparse.c index f10a61ef8..83974f846 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -561,9 +561,13 @@ static int uecc_public_key_read_binary( mbedtls_uecc_keypair *uecc_keypair, if( buf[0] != 0x04 ) return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); - mbedtls_platform_memcpy( uecc_keypair->public_key, buf + 1, 2 * NUM_ECC_BYTES ); + if( mbedtls_platform_memcpy( uecc_keypair->public_key, buf + 1, 2 * NUM_ECC_BYTES ) == + uecc_keypair->public_key ) + { + return( 0 ); + } - return( 0 ); + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } static int pk_get_ueccpubkey( unsigned char **p, @@ -976,7 +980,11 @@ static int pk_parse_key_sec1_der( mbedtls_uecc_keypair *keypair, if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, MBEDTLS_ASN1_OCTET_STRING ) ) != 0 ) return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret ); - mbedtls_platform_memcpy( keypair->private_key, p, len ); + if( mbedtls_platform_memcpy( keypair->private_key, p, len ) != + keypair->private_key ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } p += len; diff --git a/library/rsa.c b/library/rsa.c index 67ebf8424..83a4f3879 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -60,9 +60,9 @@ #include #endif -#if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" -#else + +#if !defined(MBEDTLS_PLATFORM_C) #include #define mbedtls_printf printf #define mbedtls_calloc calloc @@ -1974,8 +1974,11 @@ static int rsa_rsassa_pkcs1_v15_encode( mbedtls_md_type_t md_alg, /* Are we signing raw data? */ if( md_alg == MBEDTLS_MD_NONE ) { - mbedtls_platform_memcpy( p, hash, hashlen ); - return( 0 ); + if( mbedtls_platform_memcpy( p, hash, hashlen ) == p ) + { + return( 0 ); + } + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); } /* Signing hashed data, add corresponding ASN.1 structure @@ -2003,7 +2006,10 @@ static int rsa_rsassa_pkcs1_v15_encode( mbedtls_md_type_t md_alg, *p++ = 0x00; *p++ = MBEDTLS_ASN1_OCTET_STRING; *p++ = (unsigned char) hashlen; - mbedtls_platform_memcpy( p, hash, hashlen ); + if( mbedtls_platform_memcpy( p, hash, hashlen ) != p ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } p += hashlen; /* Just a sanity-check, should be automatic @@ -2029,7 +2035,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, const unsigned char *hash, unsigned char *sig ) { - int ret; + int ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; unsigned char *sig_try = NULL, *verif = NULL; RSA_VALIDATE_RET( ctx != NULL ); @@ -2087,7 +2093,10 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, goto cleanup; } - mbedtls_platform_memcpy( sig, sig_try, ctx->len ); + if( mbedtls_platform_memcpy( sig, sig_try, ctx->len ) != sig ) + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + } cleanup: mbedtls_free( sig_try ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index fb9539f1a..4ebfb5c93 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -92,7 +92,11 @@ int mbedtls_ssl_ecdh_read_peerkey( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - mbedtls_platform_memcpy( ssl->handshake->ecdh_peerkey, *p + 2, 2 * NUM_ECC_BYTES ); + if( mbedtls_platform_memcpy( ssl->handshake->ecdh_peerkey, *p + 2, 2 * NUM_ECC_BYTES ) != + ssl->handshake->ecdh_peerkey ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } *p += secp256r1_uncompressed_point_length; return( 0 ); @@ -1886,7 +1890,7 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, return( ret ); } - if( handshake->premaster == mbedtls_platform_zeroize( + if( handshake->premaster == mbedtls_platform_zeroize( handshake->premaster, sizeof(handshake->premaster) ) ) { return( 0 ); @@ -2293,7 +2297,10 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch if( end < p || (size_t)( end - p ) < psk_len ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - mbedtls_platform_memcpy( p, psk, psk_len ); + if( mbedtls_platform_memcpy( p, psk, psk_len ) != p ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } p += psk_len; ssl->handshake->pmslen = p - ssl->handshake->premaster; @@ -4495,8 +4502,16 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) /* Handshake hashes are computed without fragmentation, * so set frag_offset = 0 and frag_len = hs_len for now */ - mbedtls_platform_memset( ssl->out_msg + 6, 0x00, 3 ); - mbedtls_platform_memcpy( ssl->out_msg + 9, ssl->out_msg + 1, 3 ); + if( mbedtls_platform_memset( ssl->out_msg + 6, 0x00, 3 ) != + ssl->out_msg + 6 ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } + if( mbedtls_platform_memcpy( ssl->out_msg + 9, ssl->out_msg + 1, 3 ) != + ssl->out_msg + 9 ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } } #endif /* MBEDTLS_SSL_PROTO_DTLS */ @@ -6127,9 +6142,24 @@ static int ssl_buffer_message( mbedtls_ssl_context *ssl ) /* Prepare final header: copy msg_type, length and message_seq, * then add standardised fragment_offset and fragment_length */ - mbedtls_platform_memcpy( hs_buf->data, ssl->in_msg, 6 ); - mbedtls_platform_memset( hs_buf->data + 6, 0, 3 ); - mbedtls_platform_memcpy( hs_buf->data + 9, hs_buf->data + 1, 3 ); + if( mbedtls_platform_memcpy( hs_buf->data, ssl->in_msg, 6 ) != + hs_buf->data ) + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + goto exit; + } + if( mbedtls_platform_memset( hs_buf->data + 6, 0, 3 ) != + hs_buf->data + 6 ) + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + goto exit; + } + if( mbedtls_platform_memcpy( hs_buf->data + 9, hs_buf->data + 1, 3 ) != + hs_buf->data + 9 ) + { + ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED; + goto exit; + } hs_buf->is_valid = 1; @@ -11179,11 +11209,19 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) n = ( len < ssl->in_msglen ) ? len : ssl->in_msglen; - mbedtls_platform_memcpy( buf, ssl->in_offt, n ); + if( mbedtls_platform_memcpy( buf, ssl->in_offt, n ) != + buf ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } ssl->in_msglen -= n; // clear incoming data after it's copied to buffer - mbedtls_platform_memset(ssl->in_offt, 0, n); + if( mbedtls_platform_memset( ssl->in_offt, 0, n ) != + ssl->in_offt ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } if( ssl->in_msglen == 0 ) { @@ -11273,7 +11311,10 @@ static int ssl_write_real( mbedtls_ssl_context *ssl, */ ssl->out_msglen = len; ssl->out_msgtype = MBEDTLS_SSL_MSG_APPLICATION_DATA; - mbedtls_platform_memcpy( ssl->out_msg, buf, len ); + if( mbedtls_platform_memcpy( ssl->out_msg, buf, len ) != ssl->out_msg ) + { + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + } #if defined(MBEDTLS_FI_COUNTERMEASURES) && !defined(MBEDTLS_SSL_CBC_RECORD_SPLITTING) /* diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index 512686c74..6285cf357 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -81,7 +81,10 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, /* This function is designed for test purposes-only (such as validating NIST * test vectors) as it uses a provided value for d instead of generating * it uniformly at random. */ - mbedtls_platform_memcpy (_private, d, NUM_ECC_BYTES); + if( mbedtls_platform_memcpy (_private, d, NUM_ECC_BYTES) != _private ) + { + goto exit; + } /* Computing public-key from private: */ ret = EccPoint_compute_public_key(_public, _private);