Fix missing length check

There was a check against the remaining size of the buffer, which used
to be correct, but was broken two commits ago when we started not just
copying the input but also adding to it.

Replace it with a check that the input length is not greater that what's
expected for this step. This guarantees we won't overflow the internal
buffer.

While at it, add an explicit cast to uint8_t when writing the length to
the buffer, so silence an MSVC warning. This cast is safe because we
checked that the length is no larger than 65 or 32 (depending on the
step), so in any case is fits in one byte.

This was found because some lengths had not been adjusted in the test
suite, and instead of failing cleanly, library code performed buffer
overflows. I'll fix the tests in the next commit.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This commit is contained in:
Manuel Pégourié-Gonnard 2022-10-06 09:30:34 +02:00
parent 79617d99ae
commit 0771d41584

View file

@ -591,7 +591,6 @@ psa_status_t psa_pake_input( psa_pake_operation_t *operation,
{ {
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
size_t buffer_remain;
if( operation->alg == PSA_ALG_NONE || if( operation->alg == PSA_ALG_NONE ||
operation->state == PSA_PAKE_STATE_INVALID ) operation->state == PSA_PAKE_STATE_INVALID )
@ -623,6 +622,11 @@ psa_status_t psa_pake_input( psa_pake_operation_t *operation,
step != PSA_PAKE_STEP_ZK_PROOF ) step != PSA_PAKE_STEP_ZK_PROOF )
return( PSA_ERROR_INVALID_ARGUMENT ); return( PSA_ERROR_INVALID_ARGUMENT );
const psa_pake_primitive_t prim = PSA_PAKE_PRIMITIVE(
PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256 );
if( input_length > PSA_PAKE_INPUT_SIZE( PSA_ALG_JPAKE, prim, step ) )
return( PSA_ERROR_INVALID_ARGUMENT );
if( operation->state == PSA_PAKE_STATE_SETUP ) if( operation->state == PSA_PAKE_STATE_SETUP )
{ {
status = psa_pake_ecjpake_setup( operation ); status = psa_pake_ecjpake_setup( operation );
@ -660,15 +664,6 @@ psa_status_t psa_pake_input( psa_pake_operation_t *operation,
operation->sequence = PSA_PAKE_X1_STEP_KEY_SHARE; operation->sequence = PSA_PAKE_X1_STEP_KEY_SHARE;
} }
buffer_remain = MBEDTLS_PSA_PAKE_BUFFER_SIZE - operation->buffer_length;
if( input_length == 0 ||
input_length > buffer_remain )
{
psa_pake_abort( operation );
return( PSA_ERROR_INSUFFICIENT_MEMORY );
}
/* Check if step matches current sequence */ /* Check if step matches current sequence */
switch( operation->sequence ) switch( operation->sequence )
{ {
@ -719,7 +714,7 @@ psa_status_t psa_pake_input( psa_pake_operation_t *operation,
} }
/* Write the length byte */ /* Write the length byte */
operation->buffer[operation->buffer_length] = input_length; operation->buffer[operation->buffer_length] = (uint8_t) input_length;
operation->buffer_length += 1; operation->buffer_length += 1;
/* Finally copy the data */ /* Finally copy the data */