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 with modern Clang+ASan (depending on compiler optimizations).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/library/common.h b/library/common.h
index a630fcc..5359822 100644
--- a/library/common.h
+++ b/library/common.h
@@ -25,6 +25,7 @@
#include "mbedtls/build_info.h"
+#include <stddef.h>
#include <stdint.h>
/** Helper to define a function as static except when building invasive tests.
@@ -68,6 +69,42 @@
*/
#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 8c9deff..e881f2f 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -3454,8 +3454,8 @@
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 70dc74d..91a0e3b 100644
--- a/library/psa_crypto_cipher.c
+++ b/library/psa_crypto_cipher.c
@@ -516,10 +516,10 @@
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 @@
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;