Fix and simplify sign handling in mbedtls_mpi_read_string
Move the handling of the sign out of the base-specific loops. This
both simplifies the code, and corrects an edge case: the code in the
non-hexadecimal case depended on mbedtls_mpi_mul_int() preserving the
sign bit when multiplying a "negative zero" MPI by an integer, which
used to be the case but stopped with PR #2512.
Fix #4295. Thanks to Guido Vranken for analyzing the cause of the bug.
Credit to OSS-Fuzz.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/library/bignum.c b/library/bignum.c
index 56d7dbe..bfca43d 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -470,6 +470,7 @@
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t i, j, slen, n;
+ int sign = 1;
mbedtls_mpi_uint d;
mbedtls_mpi T;
MPI_VALIDATE_RET( X != NULL );
@@ -480,6 +481,12 @@
mbedtls_mpi_init( &T );
+ if( s[0] == '-' )
+ {
+ ++s;
+ sign = -1;
+ }
+
slen = strlen( s );
if( radix == 16 )
@@ -494,12 +501,6 @@
for( i = slen, j = 0; i > 0; i--, j++ )
{
- if( i == 1 && s[i - 1] == '-' )
- {
- X->s = -1;
- break;
- }
-
MBEDTLS_MPI_CHK( mpi_get_digit( &d, radix, s[i - 1] ) );
X->p[j / ( 2 * ciL )] |= d << ( ( j % ( 2 * ciL ) ) << 2 );
}
@@ -510,26 +511,15 @@
for( i = 0; i < slen; i++ )
{
- if( i == 0 && s[i] == '-' )
- {
- X->s = -1;
- continue;
- }
-
MBEDTLS_MPI_CHK( mpi_get_digit( &d, radix, s[i] ) );
MBEDTLS_MPI_CHK( mbedtls_mpi_mul_int( &T, X, radix ) );
-
- if( X->s == 1 )
- {
- MBEDTLS_MPI_CHK( mbedtls_mpi_add_int( X, &T, d ) );
- }
- else
- {
- MBEDTLS_MPI_CHK( mbedtls_mpi_sub_int( X, &T, d ) );
- }
+ MBEDTLS_MPI_CHK( mbedtls_mpi_add_int( X, &T, d ) );
}
}
+ if( sign < 0 && mbedtls_mpi_bitlen( X ) != 0 )
+ X->s = -1;
+
cleanup:
mbedtls_mpi_free( &T );