From 6692a064e3204188f686f36b62c1889ff9d58e90 Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 8 May 2020 16:57:18 +0900 Subject: [PATCH 1/4] Fix potential memory leak in EC multiplication Signed-off-by: Jonas --- library/ecp.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 104e1f122007..3c4a91aab0aa 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1544,7 +1544,10 @@ static int ecp_randomize_jac( const mbedtls_ecp_group *grp, mbedtls_ecp_point *p MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( &l, 1 ) ); if( count++ > 10 ) - return( MBEDTLS_ERR_ECP_RANDOM_FAILED ); + { + ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; + goto cleanup; + } } while( mbedtls_mpi_cmp_int( &l, 1 ) <= 0 ); @@ -2278,7 +2281,10 @@ static int ecp_randomize_mxz( const mbedtls_ecp_group *grp, mbedtls_ecp_point *P MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( &l, 1 ) ); if( count++ > 10 ) - return( MBEDTLS_ERR_ECP_RANDOM_FAILED ); + { + ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; + goto cleanup; + } } while( mbedtls_mpi_cmp_int( &l, 1 ) <= 0 ); From 923d57936921721a4f9236a858784d36a021cd77 Mon Sep 17 00:00:00 2001 From: Jonas Date: Wed, 13 May 2020 14:22:45 +0900 Subject: [PATCH 2/4] Add test cases to check rng failure Signed-off-by: Jonas --- tests/suites/test_suite_ecp.data | 8 ++++++++ tests/suites/test_suite_ecp.function | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 92191792263d..b84868c8dc78 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -446,6 +446,14 @@ ECP point multiplication Curve25519 (element of order 8) #5 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED ecp_test_mul:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660":"B8495F16056286FDB1329CEB8D09DA6AC49FF1FAE35616AEB8413B7C7AEBE0":"00":"01":"00":"01":"00":MBEDTLS_ERR_MPI_NOT_ACCEPTABLE +ECP point multiplication rng fail secp256r1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_test_mul_rng:MBEDTLS_ECP_DP_SECP256R1:"814264145F2F56F2E96A8E337A1284993FAF432A5ABCE59E867B7291D507A3AF" + +ECP point multiplication rng fail Curve25519 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_test_mul_rng:MBEDTLS_ECP_DP_CURVE25519:"5AC99F33632E5A768DE7E81BF854C27C46E3FBF2ABBACD29EC4AFF517369C660" + ECP test vectors Curve448 (RFC 7748 6.2, after decodeUCoordinate) depends_on:MBEDTLS_ECP_DP_CURVE448_ENABLED ecp_test_vec_x:MBEDTLS_ECP_DP_CURVE448:"eb7298a5c0d8c29a1dab27f1a6826300917389449741a974f5bac9d98dc298d46555bce8bae89eeed400584bb046cf75579f51d125498f98":"a01fc432e5807f17530d1288da125b0cd453d941726436c8bbd9c5222c3da7fa639ce03db8d23b274a0721a1aed5227de6e3b731ccf7089b":"ad997351b6106f36b0d1091b929c4c37213e0d2b97e85ebb20c127691d0dad8f1d8175b0723745e639a3cb7044290b99e0e2a0c27a6a301c":"0936f37bc6c1bd07ae3dec7ab5dc06a73ca13242fb343efc72b9d82730b445f3d4b0bd077162a46dcfec6f9b590bfcbcf520cdb029a8b73e":"9d874a5137509a449ad5853040241c5236395435c36424fd560b0cb62b281d285275a740ce32a22dd1740f4aa9161cec95ccc61a18f4ff07" diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 03c3e538b756..6385e7767cdb 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -724,6 +724,31 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void ecp_test_mul_rng( int id, data_t * d_hex) +{ + mbedtls_ecp_group grp; + mbedtls_mpi d; + mbedtls_ecp_point Q; + + mbedtls_ecp_group_init( &grp ); mbedtls_mpi_init( &d ); + mbedtls_ecp_point_init( &Q ); + + TEST_ASSERT( mbedtls_ecp_group_load( &grp, id ) == 0 ); + + TEST_ASSERT( mbedtls_ecp_check_pubkey( &grp, &grp.G ) == 0 ); + + TEST_ASSERT( mbedtls_mpi_read_binary( &d, d_hex->x, d_hex->len ) == 0 ); + + TEST_ASSERT( mbedtls_ecp_mul( &grp, &Q, &d, &grp.G, &rnd_zero_rand, NULL ) + == MBEDTLS_ERR_ECP_RANDOM_FAILED ); + +exit: + mbedtls_ecp_group_free( &grp ); mbedtls_mpi_free( &d ); + mbedtls_ecp_point_free( &Q ); +} +/* END_CASE */ + /* BEGIN_CASE */ void ecp_fast_mod( int id, char * N_str ) { From 4a67182962148e59f1bd26e9394901ade19ba71a Mon Sep 17 00:00:00 2001 From: Jonas Date: Wed, 13 May 2020 14:22:45 +0900 Subject: [PATCH 3/4] Add Changelog entry for #3318 Signed-off-by: Jonas --- ChangeLog.d/fix-ecp-mul-memory-leak.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/fix-ecp-mul-memory-leak.txt diff --git a/ChangeLog.d/fix-ecp-mul-memory-leak.txt b/ChangeLog.d/fix-ecp-mul-memory-leak.txt new file mode 100644 index 000000000000..2ffd51cafea1 --- /dev/null +++ b/ChangeLog.d/fix-ecp-mul-memory-leak.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix potential memory leaks in ecp_randomize_jac() and ecp_randomize_mxz() + when PRNG function fails. Contributed by Jonas Lejeune in #3317. From b246214ade8cbfa52c5168c836bd73996d0e17a6 Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 28 May 2020 20:00:43 +0900 Subject: [PATCH 4/4] Fix Changelag PR number and uniformize code when prng fails Signed-off-by: Jonas --- ChangeLog.d/fix-ecp-mul-memory-leak.txt | 2 +- library/ecp.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/fix-ecp-mul-memory-leak.txt b/ChangeLog.d/fix-ecp-mul-memory-leak.txt index 2ffd51cafea1..e82cadc2d91d 100644 --- a/ChangeLog.d/fix-ecp-mul-memory-leak.txt +++ b/ChangeLog.d/fix-ecp-mul-memory-leak.txt @@ -1,3 +1,3 @@ Bugfix * Fix potential memory leaks in ecp_randomize_jac() and ecp_randomize_mxz() - when PRNG function fails. Contributed by Jonas Lejeune in #3317. + when PRNG function fails. Contributed by Jonas Lejeune in #3318. diff --git a/library/ecp.c b/library/ecp.c index 3c4a91aab0aa..9522edf776f5 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2862,7 +2862,10 @@ int mbedtls_ecp_gen_privkey( const mbedtls_ecp_group *grp, * such as secp224k1 are actually very close to the worst case. */ if( ++count > 30 ) - return( MBEDTLS_ERR_ECP_RANDOM_FAILED ); + { + ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; + goto cleanup; + } ret = mbedtls_mpi_lt_mpi_ct( d, &grp->N, &cmp ); if( ret != 0 )