From de5eeb5ce9aa3efd4b0bdd5e003736fd0ba5cdf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Jul 2025 12:12:36 +0200 Subject: [PATCH] Relax and test aliasing rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is consistent with the general rules documented at the top of the file: - when computing GCD(A, N), there is no modular arithmetic, so the output can alias any of the inputs; - when computing a modular inverse, N is the modulus, so it can't be aliased by any of the outputs (we'll use it for modular operations over the entire course of the function's execution). But since this function has two modes of operations with different aliasing rules (G can alias N only if I == NULL), I think it should really be stated explicitly. Signed-off-by: Manuel Pégourié-Gonnard --- library/bignum_core.c | 10 ++- library/bignum_core.h | 4 +- tests/suites/test_suite_bignum_core.function | 91 ++++++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 1e80a5bf5d..e020da100e 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -1094,11 +1094,19 @@ void mbedtls_mpi_core_gcd_modinv_odd(mbedtls_mpi_uint *G, * Initial values: * u, v = A, N * q, r = 0, 1 + * + * We only write to G (aka v) after reading from inputs (A and N), which + * allows aliasing, except with N when I != NULL, as then we'll be operating + * mod N on q and r later - see the public documentation. + * + * Also avoid possible UB with memcpy when src == dst. */ memcpy(u, A, A_limbs * ciL); memset((char *) u + A_limbs * ciL, 0, (N_limbs - A_limbs) * ciL); - memcpy(v, N, N_limbs * ciL); + if (v != N) { + memcpy(v, N, N_limbs * ciL); + } if (I != NULL) { memset(q, 0, N_limbs * ciL); diff --git a/library/bignum_core.h b/library/bignum_core.h index 29e05cd3c9..1589c34640 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -826,7 +826,9 @@ void mbedtls_mpi_core_from_mont_rep(mbedtls_mpi_uint *X, * * Requires N to be odd, and 0 <= A <= N. * - * None of the parameters may alias or overlap another. + * When I == NULL (computing only the GCD), G may alias A or N. + * When I != NULL (computing the modular inverse), G or I may alias A + * but none of them may alias N (the modulus). * * \param[out] G The GCD of \p A and \p N. * Must have the same number of limbs as \p N. diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 81c2242997..93782c1331 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1416,6 +1416,38 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, mbedtls_free(T); T = NULL; + /* GCD only, G aliased to N */ + TEST_CALLOC(G, N_limbs); + memcpy(G, N, bytes); + + TEST_CALLOC(T, 4 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, A, A_limbs, G, N_limbs, T); + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + + mbedtls_free(G); + G = NULL; + mbedtls_free(T); + T = NULL; + + /* GCD only, G aliased to A (hence A_limbs = N_limbs) */ + TEST_CALLOC(G, N_limbs); + memcpy(G, A, A_limbs * sizeof(mbedtls_mpi_uint)); + + TEST_CALLOC(T, 4 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, NULL, G, N_limbs, N, N_limbs, T); + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + + mbedtls_free(G); + G = NULL; + mbedtls_free(T); + T = NULL; + /* * Test GCD + modinv */ @@ -1437,6 +1469,65 @@ void mpi_core_gcd_modinv_odd(char *input_A, char *input_N, TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); } + mbedtls_free(G); + G = NULL; + mbedtls_free(I); + I = NULL; + mbedtls_free(T); + T = NULL; + + /* GCD + modinv, G aliased to A */ + TEST_CALLOC(G, N_limbs); + memcpy(G, A, A_limbs * sizeof(mbedtls_mpi_uint)); + + TEST_CALLOC(I, N_limbs); + memset(I, 'I', bytes); + + TEST_CALLOC(T, 5 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, I, G, N_limbs, N, N_limbs, T); + + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + if (got_I) { + TEST_CF_PUBLIC(I, bytes); + TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + } + + mbedtls_free(G); + G = NULL; + mbedtls_free(I); + I = NULL; + mbedtls_free(T); + T = NULL; + + /* GCD + modinv, I aliased to A */ + TEST_CALLOC(G, N_limbs); + memset(G, 'G', bytes); + + TEST_CALLOC(I, N_limbs); + memcpy(I, A, A_limbs * sizeof(mbedtls_mpi_uint)); + + TEST_CALLOC(T, 5 * N_limbs); + memset(T, 'T', bytes); + + mbedtls_mpi_core_gcd_modinv_odd(G, I, I, N_limbs, N, N_limbs, T); + + TEST_CF_PUBLIC(G, bytes); + TEST_MEMORY_COMPARE(G, bytes, exp_G, bytes); + if (got_I) { + TEST_CF_PUBLIC(I, bytes); + TEST_MEMORY_COMPARE(I, bytes, exp_I, bytes); + } + + mbedtls_free(G); + G = NULL; + mbedtls_free(I); + I = NULL; + mbedtls_free(T); + T = NULL; + exit: mbedtls_free(A); mbedtls_free(N);