From ff6a32d79c7f30feca3cac9c3a3dc45c23c99bec Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 3 Apr 2021 20:21:43 +0200 Subject: [PATCH] Fix low-probability arithmetic error in ECC Fix the subtraction in fix_negative, which was incorrectly not looking for a carry. This caused the result to be wrong when the least significant limb of N was 0. Fix #4296. The bug was introduced by d10e8fae9e30cac60297b1e1834002db183429e5 "Optimize fix_negative". Thanks to Philippe Antoine (catenacyber) for reporting the bug which was found by his EC differential fuzzer. Credit to OSS-Fuzz. Signed-off-by: Gilles Peskine --- library/ecp_curves.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index bf84effb69..165c315d17 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -1041,12 +1041,20 @@ void mbedtls_ecp_fix_negative( mbedtls_mpi *N, signed char c, size_t bits ) { size_t i; - /* Set N := N - 2^bits */ - --N->p[0]; + /* Set N := 2^bits - 1 - N. We know that 0 <= N < 2^bits, so + * set the absolute value to 0xfff...fff - N. There is no carry + * since we're subtracting from all-bits-one. */ for( i = 0; i <= bits / 8 / sizeof( mbedtls_mpi_uint ); i++ ) { N->p[i] = ~(mbedtls_mpi_uint)0 - N->p[i]; } + /* Add 1, taking care of the carry. */ + i = 0; + do + ++N->p[i]; + while( N->p[i++] == 0 && i <= bits / 8 / sizeof( mbedtls_mpi_uint ) ); + /* Invert the sign. + * Now N = N0 - 2^bits where N0 is the initial value of N. */ N->s = -1; /* Add |c| * 2^bits to the absolute value. Since c and N are