From 0e5faf64074b987284ed3305f3bb4326662a8ad5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 8 Jun 2020 22:50:35 +0200 Subject: [PATCH] mbedtls_mpi_sub_abs: check the range of the result when it happens The function mbedtls_mpi_sub_abs first checked that A >= B and then performed the subtraction, relying on the fact that A >= B to guarantee that the carry propagation would stop, and not taking advantage of the fact that the carry when subtracting two numbers can only be 0 or 1. This made the carry propagation code a little hard to follow. Write an ad hoc loop for the carry propagation, checking the size of the result. This makes termination obvious. The initial check that A >= B is no longer needed, since the function now checks that the carry propagation terminates, which is equivalent. This is a slight performance gain. Signed-off-by: Gilles Peskine --- library/bignum.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 7a81f33b1..3825820ea 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1374,20 +1374,20 @@ static mbedtls_mpi_uint mpi_sub_hlp( size_t n, } /* - * Unsigned subtraction: X = |A| - |B| (HAC 14.9) + * Unsigned subtraction: X = |A| - |B| (HAC 14.9, 14.10) */ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *B ) { mbedtls_mpi TB; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t n; - mbedtls_mpi_uint c, z; + mbedtls_mpi_uint carry; MPI_VALIDATE_RET( X != NULL ); MPI_VALIDATE_RET( A != NULL ); MPI_VALIDATE_RET( B != NULL ); - if( mbedtls_mpi_cmp_abs( A, B ) < 0 ) - return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE ); + /* if( mbedtls_mpi_cmp_abs( A, B ) < 0 ) */ + /* return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE ); */ mbedtls_mpi_init( &TB ); @@ -1411,11 +1411,17 @@ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi if( B->p[n - 1] != 0 ) break; - c = mpi_sub_hlp( n, X->p, B->p ); - while( c != 0 ) + carry = mpi_sub_hlp( n, X->p, B->p ); + if( carry != 0 ) { - z = ( X->p[n] < c ); X->p[n] -= c; - c = z; n++; + /* Propagate the carry to the first nonzero limb of X. */ + for( ; n < X->n && X->p[n] == 0; n++ ) + --X->p[n]; + /* If we ran out of space for the carry, it means that the result + * is negative. */ + if( n == X->n ) + return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE ); + --X->p[n]; } cleanup: