From da0913ba6b3ddd5708189df8115d85bbd0ff2be3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Jun 2022 17:03:40 +0200 Subject: [PATCH] Call setbuf when reading or writing files: library After opening a file containing sensitive data, call mbedtls_setbuf() to disable buffering. This way, we don't expose sensitive data to a memory disclosure vulnerability in a buffer outside our control. This commit adds a call to mbedtls_setbuf() after each call to fopen(), except: * In ctr_drbg.c, in load_file(), because this is only used for DH parameters and they are not confidential data. * In psa_its_file.c, in psa_its_remove(), because the file is only opened to check its existence, we don't read data from it. Signed-off-by: Gilles Peskine --- library/ctr_drbg.c | 6 ++++++ library/dhm.c | 1 + library/entropy.c | 6 ++++++ library/entropy_poll.c | 5 ++++- library/hmac_drbg.c | 6 ++++++ library/md.c | 3 +++ library/pkparse.c | 3 +++ library/psa_its_file.c | 7 +++++++ 8 files changed, 36 insertions(+), 1 deletion(-) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 93a7cdcd1..a2f55cc59 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -607,6 +607,9 @@ int mbedtls_ctr_drbg_write_seed_file( mbedtls_ctr_drbg_context *ctx, if( ( f = fopen( path, "wb" ) ) == NULL ) return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( ( ret = mbedtls_ctr_drbg_random( ctx, buf, MBEDTLS_CTR_DRBG_MAX_INPUT ) ) != 0 ) goto exit; @@ -640,6 +643,9 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + n = fread( buf, 1, sizeof( buf ), f ); if( fread( &c, 1, 1, f ) != 0 ) { diff --git a/library/dhm.c b/library/dhm.c index 2ce0ed4fd..1e95bdab0 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -620,6 +620,7 @@ static int load_file( const char *path, unsigned char **buf, size_t *n ) if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_DHM_FILE_IO_ERROR ); + /* The data loaded here is public, so don't bother disabling buffering. */ fseek( f, 0, SEEK_END ); if( ( size = ftell( f ) ) == -1 ) diff --git a/library/entropy.c b/library/entropy.c index 9e31f8491..08c5bd7d1 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -457,6 +457,9 @@ int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *p goto exit; } + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( fwrite( buf, 1, MBEDTLS_ENTROPY_BLOCK_SIZE, f ) != MBEDTLS_ENTROPY_BLOCK_SIZE ) { ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; @@ -484,6 +487,9 @@ int mbedtls_entropy_update_seed_file( mbedtls_entropy_context *ctx, const char * if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + fseek( f, 0, SEEK_END ); n = (size_t) ftell( f ); fseek( f, 0, SEEK_SET ); diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 058c307df..2ae57fdc0 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -35,7 +35,7 @@ #if defined(MBEDTLS_TIMING_C) #include "mbedtls/timing.h" #endif -#if defined(MBEDTLS_ENTROPY_NV_SEED) +#if defined(MBEDTLS_ENTROPY_NV_SEED) || !defined(HAVE_SYSCTL_ARND) #include "mbedtls/platform.h" #endif @@ -195,6 +195,9 @@ int mbedtls_platform_entropy_poll( void *data, if( file == NULL ) return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( file, NULL ); + read_len = fread( output, 1, len, file ); if( read_len != len ) { diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index ab353bfd5..8b13a860f 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -436,6 +436,9 @@ int mbedtls_hmac_drbg_write_seed_file( mbedtls_hmac_drbg_context *ctx, const cha if( ( f = fopen( path, "wb" ) ) == NULL ) return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + if( ( ret = mbedtls_hmac_drbg_random( ctx, buf, sizeof( buf ) ) ) != 0 ) goto exit; @@ -465,6 +468,9 @@ int mbedtls_hmac_drbg_update_seed_file( mbedtls_hmac_drbg_context *ctx, const ch if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + n = fread( buf, 1, sizeof( buf ), f ); if( fread( &c, 1, 1, f ) != 0 ) { diff --git a/library/md.c b/library/md.c index f2c1a90f8..a387da50a 100644 --- a/library/md.c +++ b/library/md.c @@ -605,6 +605,9 @@ int mbedtls_md_file( const mbedtls_md_info_t *md_info, const char *path, unsigne if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_MD_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + mbedtls_md_init( &ctx ); if( ( ret = mbedtls_md_setup( &ctx, md_info, 0 ) ) != 0 ) diff --git a/library/pkparse.c b/library/pkparse.c index 22dab3ad7..0e5dd5546 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -82,6 +82,9 @@ int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n ) if( ( f = fopen( path, "rb" ) ) == NULL ) return( MBEDTLS_ERR_PK_FILE_IO_ERROR ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( f, NULL ); + fseek( f, 0, SEEK_END ); if( ( size = ftell( f ) ) == -1 ) { diff --git a/library/psa_its_file.c b/library/psa_its_file.c index f05872095..b7c2e6b04 100644 --- a/library/psa_its_file.c +++ b/library/psa_its_file.c @@ -102,6 +102,9 @@ static psa_status_t psa_its_read_file( psa_storage_uid_t uid, if( *p_stream == NULL ) return( PSA_ERROR_DOES_NOT_EXIST ); + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( *p_stream, NULL ); + n = fread( &header, 1, sizeof( header ), *p_stream ); if( n != sizeof( header ) ) return( PSA_ERROR_DATA_CORRUPT ); @@ -201,9 +204,13 @@ psa_status_t psa_its_set( psa_storage_uid_t uid, psa_its_fill_filename( uid, filename ); stream = fopen( PSA_ITS_STORAGE_TEMP, "wb" ); + if( stream == NULL ) goto exit; + /* Ensure no stdio buffering of secrets, as such buffers cannot be wiped. */ + mbedtls_setbuf( stream, NULL ); + status = PSA_ERROR_INSUFFICIENT_STORAGE; n = fwrite( &header, 1, sizeof( header ), stream ); if( n != sizeof( header ) )