From 771c47055f2b9db6762ab32dab36092952990573 Mon Sep 17 00:00:00 2001 From: Minos Galanakis Date: Thu, 27 Oct 2022 12:22:22 +0100 Subject: [PATCH] bignum_mod: Style changes This patch addresses review comments with regards to style of `mbedtls_mpi_mod_modulus_setup/free()`. It also removes a test check which was triggering a use-after-free. Signed-off-by: Minos Galanakis --- library/bignum_mod.c | 9 +++++---- tests/suites/test_suite_bignum_mod.function | 7 ------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/library/bignum_mod.c b/library/bignum_mod.c index 92c011cffe..705b2dbae6 100644 --- a/library/bignum_mod.c +++ b/library/bignum_mod.c @@ -81,7 +81,8 @@ void mbedtls_mpi_mod_modulus_free( mbedtls_mpi_mod_modulus *m ) m->limbs ); mbedtls_free( (mbedtls_mpi_uint *)m->rep.mont.rr ); m->rep.mont.rr = NULL; - m->rep.mont.mm = 0; break; + m->rep.mont.mm = 0; + break; case MBEDTLS_MPI_MOD_REP_OPT_RED: mbedtls_free( m->rep.ored ); break; @@ -110,11 +111,11 @@ static int set_mont_const_square( const mbedtls_mpi_uint **X, if ( A == NULL || limbs == 0 || limbs >= ( MBEDTLS_MPI_MAX_LIMBS / 2 ) - 2 ) goto cleanup; - if ( !mbedtls_mpi_grow( &N, limbs )) - memcpy( N.p, A, sizeof(mbedtls_mpi_uint) * limbs ); - else + if ( mbedtls_mpi_grow( &N, limbs )) goto cleanup; + memcpy( N.p, A, sizeof(mbedtls_mpi_uint) * limbs ); + mbedtls_mpi_core_get_mont_r2_unsafe(&RR, &N); *X = RR.p; diff --git a/tests/suites/test_suite_bignum_mod.function b/tests/suites/test_suite_bignum_mod.function index 8716f679a3..2e0377a703 100644 --- a/tests/suites/test_suite_bignum_mod.function +++ b/tests/suites/test_suite_bignum_mod.function @@ -18,7 +18,6 @@ void mpi_mod_setup( int ext_rep, int int_rep, int iret ) mbedtls_mpi_uint mp[MLIMBS]; mbedtls_mpi_mod_modulus m; const size_t mp_size = sizeof(mbedtls_mpi_uint); - mbedtls_mpi_uint * rr; int ret; memset( mp, 0xFF, mp_size ); @@ -34,8 +33,6 @@ void mpi_mod_setup( int ext_rep, int int_rep, int iret ) TEST_ASSERT( m.rep.mont.rr != NULL ); TEST_ASSERT( m.rep.mont.mm != 0 ); - /* Keep a copy of the memory location used to store Montgomery const */ - rr = (mbedtls_mpi_uint *)m.rep.mont.rr; } /* Address sanitiser should catch if we try to free mp */ @@ -54,10 +51,6 @@ void mpi_mod_setup( int ext_rep, int int_rep, int iret ) TEST_ASSERT( m.rep.mont.rr == NULL ); TEST_ASSERT( m.rep.mont.mm == 0 ); - /* mbedtls_mpi_mod_modulus_free() has set the - * (mbedtls_mpi_uint *)m.rep.mont.rr -> NULL. - * Verify that the actual data have been zeroed */ - ASSERT_COMPARE( rr, mp_size, &mp, mp_size ); } exit: /* It should be safe to call an mbedtls free several times */