From bc0ff75455f64f784be9eb48c15ccb0b8d868465 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 22 Oct 2022 19:57:16 +0200 Subject: [PATCH] Fix NULL+0 undefined behavior in ECB encryption and decryption psa_cipher_encrypt() and psa_cipher_decrypt() sometimes add a zero offset to a null pointer when the cipher does not use an IV. This is undefined behavior, although it works as naively expected on most platforms. This can cause a crash under modern Asan (depending on compiler optimizations). Signed-off-by: Gilles Peskine --- ChangeLog.d/psa-ecb-ub.txt | 3 +++ library/psa_crypto.c | 3 ++- library/psa_crypto_cipher.c | 11 ++++++++--- tests/suites/test_suite_psa_crypto.function | 4 ++-- 4 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 ChangeLog.d/psa-ecb-ub.txt diff --git a/ChangeLog.d/psa-ecb-ub.txt b/ChangeLog.d/psa-ecb-ub.txt new file mode 100644 index 0000000000..9d725ac706 --- /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/psa_crypto.c b/library/psa_crypto.c index 2ce5e4320d..093940f705 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3467,7 +3467,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 == NULL ? NULL : output + default_iv_length ), + output_size - default_iv_length, output_length ); exit: diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 70dc74d748..5f6c7c9bb5 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -517,7 +517,8 @@ psa_status_t mbedtls_psa_cipher_encrypt( goto exit; status = mbedtls_psa_cipher_finish( &operation, - output + update_output_length, + ( output == NULL ? NULL : + output + update_output_length ), output_size - update_output_length, &finish_output_length ); if( status != PSA_SUCCESS ) @@ -563,7 +564,9 @@ psa_status_t mbedtls_psa_cipher_decrypt( goto exit; } - status = mbedtls_psa_cipher_update( &operation, input + operation.iv_length, + status = mbedtls_psa_cipher_update( &operation, + ( input == NULL ? NULL : + input + operation.iv_length ), input_length - operation.iv_length, output, output_size, &olength ); if( status != PSA_SUCCESS ) @@ -571,7 +574,9 @@ psa_status_t mbedtls_psa_cipher_decrypt( accumulated_length = olength; - status = mbedtls_psa_cipher_finish( &operation, output + accumulated_length, + status = mbedtls_psa_cipher_finish( &operation, + ( output == NULL ? NULL : + output + accumulated_length ), output_size - accumulated_length, &olength ); if( status != PSA_SUCCESS ) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 779f594dca..e1f96c0ea8 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3962,7 +3962,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, + output == NULL ? NULL : output + output_length, output_buffer_size - output_length, &length ) ); output_length += length; @@ -3980,7 +3980,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, + output == NULL ? NULL : output + output_length, output_buffer_size - output_length, &length ) ); output_length += length;