Handle null pointers safely when used as buffers of size 0
When the size of a buffer is 0, the corresponding pointer argument may
be null. In such cases, library functions must not perform arithmetic
on the pointer or call standard library functions such as memset and
memcpy, since that would be undefined behavior in C. Protect such
cases.
Refactor the storage of a 0-sized raw data object to make it store a
null pointer, rather than depending on the behavior of calloc(1,0).
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 4a33639..19db5a9 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -409,10 +409,17 @@
switch( type )
{
case PSA_KEY_TYPE_RAW_DATA:
+ if( bits == 0 )
+ {
+ raw->bytes = 0;
+ raw->data = NULL;
+ return( PSA_SUCCESS );
+ }
+ break;
#if defined(MBEDTLS_MD_C)
case PSA_KEY_TYPE_HMAC:
-#endif
break;
+#endif
#if defined(MBEDTLS_AES_C)
case PSA_KEY_TYPE_AES:
if( bits != 128 && bits != 192 && bits != 256 )
@@ -478,7 +485,8 @@
&slot->data.raw );
if( status != PSA_SUCCESS )
return( status );
- memcpy( slot->data.raw.data, data, data_length );
+ if( data_length != 0 )
+ memcpy( slot->data.raw.data, data, data_length );
}
else
#if defined(MBEDTLS_PK_PARSE_C)
@@ -679,7 +687,8 @@
{
if( slot->data.raw.bytes > data_size )
return( PSA_ERROR_BUFFER_TOO_SMALL );
- memcpy( data, slot->data.raw.data, slot->data.raw.bytes );
+ if( slot->data.raw.bytes != 0 )
+ memcpy( data, slot->data.raw.data, slot->data.raw.bytes );
*data_length = slot->data.raw.bytes;
return( PSA_SUCCESS );
}
@@ -710,7 +719,10 @@
ret = mbedtls_pk_write_key_der( &pk, data, data_size );
if( ret < 0 )
{
- memset( data, 0, data_size );
+ /* If data_size is 0 then data may be NULL and then the
+ * call to memset would have undefined behavior. */
+ if( data_size != 0 )
+ memset( data, 0, data_size );
return( mbedtls_to_psa_error( ret ) );
}
/* The mbedtls_pk_xxx functions write to the end of the buffer.
@@ -999,7 +1011,10 @@
* (barring an attack on the hash and deliberately-crafted input),
* in case the caller doesn't check the return status properly. */
*hash_length = actual_hash_length;
- memset( hash, '!', hash_size );
+ /* If hash_size is 0 then hash may be NULL and then the
+ * call to memset would have undefined behavior. */
+ if( hash_size != 0 )
+ memset( hash, '!', hash_size );
if( hash_size < actual_hash_length )
return( PSA_ERROR_BUFFER_TOO_SMALL );
@@ -1500,7 +1515,10 @@
* (barring an attack on the mac and deliberately-crafted input),
* in case the caller doesn't check the return status properly. */
*mac_length = operation->mac_size;
- memset( mac, '!', mac_size );
+ /* If mac_size is 0 then mac may be NULL and then the
+ * call to memset would have undefined behavior. */
+ if( mac_size != 0 )
+ memset( mac, '!', mac_size );
if( mac_size < operation->mac_size )
return( PSA_ERROR_BUFFER_TOO_SMALL );
@@ -1944,8 +1962,10 @@
if( status == PSA_SUCCESS )
memset( signature + *signature_length, '!',
signature_size - *signature_length );
- else
+ else if( signature_size != 0 )
memset( signature, '!', signature_size );
+ /* If signature_size is 0 then we have nothing to do. We must not call
+ * memset because signature may be NULL in this case. */
return( status );
}
@@ -2410,7 +2430,9 @@
psa_cipher_abort( operation );
return( mbedtls_to_psa_error( ret ) );
}
- if( output_size >= *output_length )
+ if( *output_length == 0 )
+ /* Nothing to copy. Note that output may be NULL in this case. */ ;
+ else if( output_size >= *output_length )
memcpy( output, temp_output_buffer, *output_length );
else
{
@@ -2684,7 +2706,10 @@
if( ret != 0 )
{
- memset( ciphertext, 0, ciphertext_size );
+ /* If ciphertext_size is 0 then ciphertext may be NULL and then the
+ * call to memset would have undefined behavior. */
+ if( ciphertext_size != 0 )
+ memset( ciphertext, 0, ciphertext_size );
return( mbedtls_to_psa_error( ret ) );
}
@@ -2823,7 +2848,12 @@
}
if( ret != 0 )
- memset( plaintext, 0, plaintext_size );
+ {
+ /* If plaintext_size is 0 then plaintext may be NULL and then the
+ * call to memset has undefined behavior. */
+ if( plaintext_size != 0 )
+ memset( plaintext, 0, plaintext_size );
+ }
else
*plaintext_length = ciphertext_length - tag_length;