diff --git a/ChangeLog.d/psa-ecb-ub.txt b/ChangeLog.d/psa-ecb-ub.txt new file mode 100644 index 000000000..9d725ac70 --- /dev/null +++ b/ChangeLog.d/psa-ecb-ub.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix undefined behavior (typically harmless in practice) in PSA ECB + encryption and decryption. diff --git a/library/common.h b/library/common.h index a630fcc45..53598228b 100644 --- a/library/common.h +++ b/library/common.h @@ -25,6 +25,7 @@ #include "mbedtls/build_info.h" +#include #include /** Helper to define a function as static except when building invasive tests. @@ -68,6 +69,42 @@ extern void (*mbedtls_test_hook_test_fail)( const char * test, int line, const c */ #define MBEDTLS_ALLOW_PRIVATE_ACCESS +/** Return an offset into a buffer. + * + * This is just the addition of an offset to a pointer, except that this + * function also accepts an offset of 0 into a buffer whose pointer is null. + * + * \param p Pointer to a buffer of at least n bytes. + * This may be \p NULL if \p n is zero. + * \param n An offset in bytes. + * \return Pointer to offset \p n in the buffer \p p. + * Note that this is only a valid pointer if the size of the + * buffer is at least \p n + 1. + */ +static inline unsigned char *mbedtls_buffer_offset( + unsigned char *p, size_t n ) +{ + return( p == NULL ? NULL : p + n ); +} + +/** Return an offset into a read-only buffer. + * + * This is just the addition of an offset to a pointer, except that this + * function also accepts an offset of 0 into a buffer whose pointer is null. + * + * \param p Pointer to a buffer of at least n bytes. + * This may be \p NULL if \p n is zero. + * \param n An offset in bytes. + * \return Pointer to offset \p n in the buffer \p p. + * Note that this is only a valid pointer if the size of the + * buffer is at least \p n + 1. + */ +static inline const unsigned char *mbedtls_buffer_offset_const( + const unsigned char *p, size_t n ) +{ + return( p == NULL ? NULL : p + n ); +} + /** Byte Reading Macros * * Given a multi-byte integer \p x, MBEDTLS_BYTE_n retrieves the n-th diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8c9deffad..e881f2f3c 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3454,8 +3454,8 @@ psa_status_t psa_cipher_encrypt( mbedtls_svc_key_id_t key, status = psa_driver_wrapper_cipher_encrypt( &attributes, slot->key.data, slot->key.bytes, alg, local_iv, default_iv_length, input, input_length, - output + default_iv_length, output_size - default_iv_length, - output_length ); + mbedtls_buffer_offset( output, default_iv_length ), + output_size - default_iv_length, output_length ); exit: unlock_status = psa_unlock_key_slot( slot ); diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 70dc74d74..91a0e3b30 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -516,10 +516,10 @@ psa_status_t mbedtls_psa_cipher_encrypt( if( status != PSA_SUCCESS ) goto exit; - status = mbedtls_psa_cipher_finish( &operation, - output + update_output_length, - output_size - update_output_length, - &finish_output_length ); + status = mbedtls_psa_cipher_finish( + &operation, + mbedtls_buffer_offset( output, update_output_length ), + output_size - update_output_length, &finish_output_length ); if( status != PSA_SUCCESS ) goto exit; @@ -563,17 +563,20 @@ psa_status_t mbedtls_psa_cipher_decrypt( goto exit; } - status = mbedtls_psa_cipher_update( &operation, input + operation.iv_length, - input_length - operation.iv_length, - output, output_size, &olength ); + status = mbedtls_psa_cipher_update( + &operation, + mbedtls_buffer_offset_const( input, operation.iv_length ), + input_length - operation.iv_length, + output, output_size, &olength ); if( status != PSA_SUCCESS ) goto exit; accumulated_length = olength; - status = mbedtls_psa_cipher_finish( &operation, output + accumulated_length, - output_size - accumulated_length, - &olength ); + status = mbedtls_psa_cipher_finish( + &operation, + mbedtls_buffer_offset( output, accumulated_length ), + output_size - accumulated_length, &olength ); if( status != PSA_SUCCESS ) goto exit; diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index ca1614bef..1f3b3b64a 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -4,6 +4,7 @@ #include "mbedtls/asn1.h" #include "mbedtls/asn1write.h" #include "mbedtls/oid.h" +#include "common.h" /* For MBEDTLS_CTR_DRBG_MAX_REQUEST, knowing that psa_generate_random() * uses mbedtls_ctr_drbg internally. */ @@ -3983,7 +3984,7 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data, TEST_LE_U( length, output_buffer_size ); output_length += length; PSA_ASSERT( psa_cipher_finish( &operation, - output + output_length, + mbedtls_buffer_offset( output, output_length ), output_buffer_size - output_length, &length ) ); output_length += length; @@ -4001,7 +4002,7 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data, TEST_LE_U( length, output_buffer_size ); output_length += length; PSA_ASSERT( psa_cipher_finish( &operation, - output + output_length, + mbedtls_buffer_offset( output, output_length ), output_buffer_size - output_length, &length ) ); output_length += length;