From 0771d41584fc9c72514e798624473fe1ef6d3318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Oct 2022 09:30:34 +0200 Subject: [PATCH] Fix missing length check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- library/psa_crypto_pake.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index a8b02e396..262d44d20 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -591,7 +591,6 @@ psa_status_t psa_pake_input( psa_pake_operation_t *operation, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - size_t buffer_remain; if( operation->alg == PSA_ALG_NONE || 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 ) 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 ) { 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; } - 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 */ switch( operation->sequence ) { @@ -719,7 +714,7 @@ psa_status_t psa_pake_input( psa_pake_operation_t *operation, } /* Write the length byte */ - operation->buffer[operation->buffer_length] = input_length; + operation->buffer[operation->buffer_length] = (uint8_t) input_length; operation->buffer_length += 1; /* Finally copy the data */