Skip to content

Check the ECC scalar length without timing that depends on its top bit, and clear the mp_int in mp_rand #12129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dgarske wants to merge 2 commits into
base: master
Choose a base branch
from
+151 −14
Open

Check the ECC scalar length without timing that depends on its top bit, and clear the mp_int in mp_rand #12129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7828e53
Check the ECC scalar length without timing that depends on its top bit
dgarske Nov 13, 2026
d4ffd30
Clear the mp_int in mp_rand so a wider prior value cannot survive
dgarske Nov 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions tests/api/test_ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4427,6 +4427,108 @@ int test_wc_ecc_mulmod_unreduced(void)
return EXPECT_RESULT();
} /* END test_wc_ecc_mulmod_unreduced */

#if defined(HAVE_ECC) && defined(WOLFSSL_PUBLIC_MP) && \
!defined(NO_ECC_SECP) && \
!(defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \
defined(WOLFSSL_MICROCHIP_TA100)) && \
!defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(HAVE_SELFTEST) && \
!defined(HAVE_FIPS) && !defined(WOLFSSL_NO_ECC_SW)
#define TEST_ECC_MULMOD_K_LEN
#endif

#ifdef TEST_ECC_MULMOD_K_LEN
/* Set k to 2^bit. */
static int test_ecc_k_pow2(mp_int* k, int bit)
{
byte buf[80];
int len = bit / 8 + 1;

if ((bit < 0) || (len > (int)sizeof(buf))) {
return BAD_FUNC_ARG;
}
XMEMSET(buf, 0, sizeof(buf));
buf[0] = (byte)(1 << (bit % 8));
return mp_read_unsigned_bin(k, buf, (word32)len);
}

/* Check the scalar length limit of wc_ecc_mulmod() on one curve. */
static int test_ecc_mulmod_k_len_curve(int curveId)

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Nov 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static test helper unused when no per-curve guard holds, breaking -Werror builds · Dead/unreachable code

Static test_ecc_mulmod_k_len_curve is guarded only by TEST_ECC_MULMOD_K_LEN, which has no curve conditions. With WOLFSSL_SP_MATH+WOLFSSL_SP_NO_256 and no WOLFSSL_SP_521, or P-384-only ECC_USER_CURVES, both call sites drop out and -Wunused-function under -Werror fails the build.

Suggested fix: Add the OR of the P-256 and P-521 call-site conditions to the TEST_ECC_MULMOD_K_LEN guard, as test_ecc_import_compressed_no_root does at line 1501.
Basis: GCC manual, -Wunused-function: warns whenever a non-inline static function is unused and is enabled by -Wall; wolfSSL's AX_HARDEN_CC_COMPILER_FLAGS adds -Werror in VCS checkouts.

{
EXPECT_DECLS;
const ecc_set_type* dp = NULL;
ecc_point* g = NULL;
ecc_point* r = NULL;
mp_int a;
mp_int prime;
mp_int k;
int bits = 0;

XMEMSET(&a, 0, sizeof(mp_int));
XMEMSET(&prime, 0, sizeof(mp_int));
XMEMSET(&k, 0, sizeof(mp_int));

ExpectNotNull(dp = wc_ecc_get_curve_params(
wc_ecc_get_curve_idx(curveId)));
ExpectIntEQ(mp_init_multi(&a, &prime, &k, NULL, NULL, NULL), 0);
ExpectNotNull(g = wc_ecc_new_point());
ExpectNotNull(r = wc_ecc_new_point());
if ((dp != NULL) && (g != NULL)) {
ExpectIntEQ(mp_read_radix(&a, dp->Af, MP_RADIX_HEX), 0);
ExpectIntEQ(mp_read_radix(&prime, dp->prime, MP_RADIX_HEX), 0);
ExpectIntEQ(mp_read_radix(g->x, dp->Gx, MP_RADIX_HEX), 0);
ExpectIntEQ(mp_read_radix(g->y, dp->Gy, MP_RADIX_HEX), 0);
ExpectIntEQ(mp_set(g->z, 1), 0);
bits = mp_count_bits(&prime);
}

/* A scalar one bit shorter than the prime is accepted. */
ExpectIntEQ(test_ecc_k_pow2(&k, bits - 1), 0);
ExpectIntEQ(wc_ecc_mulmod(&k, g, r, &a, &prime, 1), 0);
/* Two bits longer than the prime is rejected. One bit longer is not
* checked: some paths allow it. */
ExpectIntEQ(test_ecc_k_pow2(&k, bits + 1), 0);
ExpectIntEQ(wc_ecc_mulmod(&k, g, r, &a, &prime, 1),
WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));
/* A bit in a digit above the prime's top digit is rejected. */
ExpectIntEQ(test_ecc_k_pow2(&k, bits + 64), 0);
ExpectIntEQ(wc_ecc_mulmod(&k, g, r, &a, &prime, 1),
WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E));

wc_ecc_del_point(r);
wc_ecc_del_point(g);
mp_free(&k);
mp_free(&prime);
mp_free(&a);

return EXPECT_RESULT();
}
#endif

/*
* Testing the scalar length limit of wc_ecc_mulmod() around the bit length of
* the prime.
*/
int test_wc_ecc_mulmod_k_len(void)
{
EXPECT_DECLS;
#ifdef TEST_ECC_MULMOD_K_LEN
#if (!defined(NO_ECC256) || defined(HAVE_ALL_CURVES)) && \
(ECC_MIN_KEY_SZ <= 256) && \
(!defined(WOLFSSL_SP_MATH) || !defined(WOLFSSL_SP_NO_256))
ExpectIntEQ(test_ecc_mulmod_k_len_curve(ECC_SECP256R1), TEST_SUCCESS);
#endif
#if (defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)) && \
(ECC_MIN_KEY_SZ <= 521) && \
(!defined(WOLFSSL_SP_MATH) || defined(WOLFSSL_SP_521))
ExpectIntEQ(test_ecc_mulmod_k_len_curve(ECC_SECP521R1), TEST_SUCCESS);
#endif
#ifdef FP_ECC
wc_ecc_fp_free();
#endif
#endif
return EXPECT_RESULT();
} /* END test_wc_ecc_mulmod_k_len */

#if defined(HAVE_ECC) && defined(WOLFSSL_PUBLIC_MP) && \
defined(WOLFSSL_PUBLIC_ECC_ADD_DBL) && !defined(WOLFSSL_SP_MATH) && \
!defined(NO_ECC256) && !defined(NO_ECC_SECP) && \
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_ecc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ int test_wc_ecc_shared_secret_ssh(void);
int test_wc_ecc_verify_hash_ex(void);
int test_wc_ecc_mulmod(void);
int test_wc_ecc_mulmod_unreduced(void);
int test_wc_ecc_mulmod_k_len(void);
int test_wc_ecc_mulmod_zero(void);
int test_wc_ecc_mulmod_negative(void);
int test_wc_ecc_mulmod_curves(void);
Expand Down Expand Up @@ -149,6 +150,7 @@ int test_wc_ecc_make_key_id_dev(void);
TEST_DECL_GROUP("ecc", test_wc_ecc_verify_hash_ex), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_unreduced), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_k_len), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_zero), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_negative), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_curves), \
Expand Down
8 changes: 8 additions & 0 deletions tests/api/test_wolfmath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ int test_mp_rand(void)
mp_int a;
WC_RNG rng;
int digits = 1;
byte wide[5 * sizeof(mp_digit)];

XMEMSET(&a, 0, sizeof(mp_int));
XMEMSET(&rng, 0, sizeof(WC_RNG));
Expand All @@ -372,6 +373,13 @@ int test_mp_rand(void)
ExpectIntNE(mp_rand(&a, 100000, &rng), 0);
#endif
ExpectIntEQ(mp_rand(&a, digits, &rng), 0);
/* A wider value already in a must not survive into the result. */
XMEMSET(wide, 0xff, sizeof(wide));
ExpectIntEQ(mp_read_unsigned_bin(&a, wide, (word32)sizeof(wide)),
MP_OKAY);
ExpectIntEQ(mp_rand(&a, 2, &rng), 0);
ExpectIntLE(mp_count_bits(&a), 2 * DIGIT_BIT);
ExpectIntGT(mp_count_bits(&a), DIGIT_BIT);

mp_clear(&a);
DoExpectIntEQ(wc_FreeRng(&rng), 0);
Expand Down
50 changes: 37 additions & 13 deletions wolfcrypt/src/ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,30 @@ static void alt_fp_init(mp_int* a)
}
#endif /* ALT_ECC_SIZE */

/* Returns 1 when the scalar k has more than bits bits, otherwise 0.
*
* mp_count_bits() takes time that depends on the position of the top set bit.
* This ORs together every bit of k at or above bits instead, so only the
* number of digits in k decides the time taken.
*/
static WC_MAYBE_UNUSED int ecc_k_too_long(const mp_int* k, int bits)
{
int i;
int top = bits / DIGIT_BIT;
int n = mp_get_digit_count(k);
mp_digit d = 0;

for (i = top; i < n; i++) {
mp_digit v = mp_get_digit(k, i);
if (i == top) {
v = (mp_digit)(v >> (bits % DIGIT_BIT));
}
d |= v;
}

return d != 0;
}


#if !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \
!defined(WOLFSSL_NO_ECC_SW) && \
Expand Down Expand Up @@ -3889,7 +3913,7 @@ static int ecc_sp_mulmod(const mp_int* k, ecc_point* G, ecc_point* R,
* and y only. It writes the result's digits but not their signs, so the
* result point's ordinates must not be negative either. Leave anything
* else to the generic code, which handles them. */
if (mp_isneg(k) || (mp_count_bits(k) > bits)) {
if (mp_isneg(k) || ecc_k_too_long(k, bits)) {
return NOT_COMPILED_IN;
}
if (mp_isneg(G->x) || mp_isneg(G->y) ||
Expand Down Expand Up @@ -3992,7 +4016,7 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
}

/* k can't be negative or have more bits than modulus count plus 1 */
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus) + 1)) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(modulus) + 1)) {
err = ECC_OUT_OF_RANGE_E;
goto exit;
}
Expand Down Expand Up @@ -4096,7 +4120,7 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
/* For supported curves the order is the same length in bits as the modulus.
* Can't have more than order bits for the scalar.
*/
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus))) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(modulus))) {
return ECC_OUT_OF_RANGE_E;
}
if (mp_count_bits(G->x) > mp_count_bits(modulus) ||
Expand Down Expand Up @@ -4224,14 +4248,14 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point* G, ecc_point* R, mp_int* a,

#ifdef HAVE_ECC_CDH
if (mp_count_bits(modulus) > mp_count_bits(order)) {
if (mp_count_bits(k) > mp_count_bits(modulus)) {
if (ecc_k_too_long(k, mp_count_bits(modulus))) {
return ECC_OUT_OF_RANGE_E;
}
}
else
#endif
/* k can't have more bits than order */
if (mp_count_bits(k) > mp_count_bits(order)) {
if (ecc_k_too_long(k, mp_count_bits(order))) {
WOLFSSL_MSG("Private key length is greater than order in bits.");
return ECC_OUT_OF_RANGE_E;
}
Expand Down Expand Up @@ -4325,7 +4349,7 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point* G, ecc_point* R, mp_int* a,
if (k == NULL || G == NULL || R == NULL || modulus == NULL) {
return ECC_BAD_ARG_E;
}
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus))) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(modulus))) {
return ECC_OUT_OF_RANGE_E;
}
if (mp_count_bits(G->x) > mp_count_bits(modulus) ||
Expand Down Expand Up @@ -13569,7 +13593,7 @@ int wc_ecc_sig_size(const ecc_key* key)
#define FP_COMB_MAX_MOD_SZ 126
#endif
/* The comb covers the bits of the modulus. An order can be a bit longer. */
#define FP_K_FITS_COMB(k, m) ((mp_count_bits(k) <= mp_count_bits(m)) && \
#define FP_K_FITS_COMB(k, m) (!ecc_k_too_long(k, mp_count_bits(m)) && \
(mp_unsigned_bin_size(m) <= FP_COMB_MAX_MOD_SZ))
#else
#define FP_MULMOD_USES_LUT 1
Expand Down Expand Up @@ -14589,7 +14613,7 @@ static int accel_fp_mul(int idx, const mp_int* k, ecc_point *R, mp_int* a,
#endif

/* if it's smaller than modulus we fine */
if (mp_unsigned_bin_size(k) > mp_unsigned_bin_size(modulus)) {
if (ecc_k_too_long(k, mp_unsigned_bin_size(modulus) * 8)) {
/* find order */
y = (unsigned)mp_unsigned_bin_size(modulus);
for (x = 0; ecc_sets[x].size; x++) {
Expand Down Expand Up @@ -14621,7 +14645,7 @@ static int accel_fp_mul(int idx, const mp_int* k, ecc_point *R, mp_int* a,
lut_gap = bitlen / FP_LUT;

/* get the k value */
if (mp_unsigned_bin_size(tk) > (int)(KB_SIZE - 2)) {
if (ecc_k_too_long(tk, (int)(KB_SIZE - 2) * 8)) {
err = BUFFER_E; goto done;
}
#ifdef ECC_TIMING_RESISTANT
Expand Down Expand Up @@ -15230,7 +15254,7 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
}

/* k can't be negative or have more bits than modulus count plus 1 */
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus) + 1)) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(modulus) + 1)) {
return ECC_OUT_OF_RANGE_E;
}

Expand Down Expand Up @@ -15329,7 +15353,7 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
if (k == NULL || G == NULL || R == NULL || a == NULL || modulus == NULL) {
return ECC_BAD_ARG_E;
}
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus))) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(modulus))) {
return ECC_OUT_OF_RANGE_E;
}
if (mp_count_bits(G->x) > mp_count_bits(modulus) ||
Expand Down Expand Up @@ -15398,7 +15422,7 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
}

/* k can't be negative or have more bits than order */
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(order))) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(order))) {
return ECC_OUT_OF_RANGE_E;
}

Expand Down Expand Up @@ -15500,7 +15524,7 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
order == NULL) {
return ECC_BAD_ARG_E;
}
if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus))) {
if (mp_isneg(k) || ecc_k_too_long(k, mp_count_bits(modulus))) {
return ECC_OUT_OF_RANGE_E;
}
if (mp_count_bits(G->x) > mp_count_bits(modulus) ||
Expand Down
3 changes: 2 additions & 1 deletion wolfcrypt/src/wolfmath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ int mp_rand(mp_int* a, int digits, WC_RNG* rng)
#ifdef USE_INTEGER_HEAP_MATH
/* allocate space for digits */
if (ret == MP_OKAY) {
ret = mp_set_bit(a, digits * DIGIT_BIT - 1);
/* Zeroed first, so no wider value already in a survives. */
ret = mp_2expt(a, digits * DIGIT_BIT - 1);
}
#else
#if defined(WOLFSSL_SP_MATH) || defined(WOLFSSL_SP_MATH_ALL)
Expand Down
Loading