From bef27f12405ee2b79394e2b286e6c80e0cd7b618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Thu, 5 Nov 2026 18:06:34 +0100 Subject: [PATCH] Reject negative scalars in ECC point multiplication wc_ecc_mulmod_ex(), wc_ecc_mulmod_ex2() and ecc_mul2add() checked the scalar only by bit length and then read its magnitude, so a negative k computed |k|*G and returned success. That is the negation of the right result. Negative values reach these functions with fastmath, heap math and sp_int with WOLFSSL_SP_INT_NEGATIVE, which every OPENSSL_EXTRA build defines. Return ECC_OUT_OF_RANGE_E for a negative scalar at every entry point, as ecc_make_pub_sw() and the signature range checks already do, including the NXP LTC and STM32 PKA ports, which have their own. Those ports set the sign of the result's y from k; they now set it positive, since reading the coordinates leaves the sign of a reused point unchanged. The fixed-point ecc_mul2add() also gains the argument checks the other implementation has, done before it allocates. The SP-only wc_ecc_mulmod_ex2() and the fixed-point SP-only wc_ecc_mulmod_ex() and wc_ecc_mulmod_ex2() also check the scalar's length, as the SP-only wc_ecc_mulmod_ex() does. The SP code drops the high digits of a longer scalar and returned a wrong point. The LTC port checks the length as the generic code does, rejects point coordinates wider than the prime with IS_POINT_E, and refuses a prime wider than its coordinate buffers, since it copied all of them into fixed stack buffers. The STM32 PKA port checks the length too, so it returns ECC_OUT_OF_RANGE_E rather than BUFFER_E. EC_POINT_mul() passed the BIGNUM's mp_int straight through. OpenSSL reduces a negative or over-wide scalar modulo the number of points on the curve, the order times the cofactor. Reduce n and m the same way when they are negative or wider than the prime, which some backends reject, so they give the same result. Some math libraries leave the remainder with the sign of the dividend, so a negative remainder gets the modulus added. Reducing by the order alone would be wrong for a point outside the prime-order subgroup on a curve with a cofactor. The bound comes from the curve's degree, so scalars in range cost nothing extra, and the modulus is computed once. Fixes F-16901. --- doc/dox_comments/header_files/ecc.h | 1 + src/pk_ec.c | 64 +++++++++++++++++ tests/api/test_ecc.c | 104 ++++++++++++++++++++++++++++ tests/api/test_ecc.h | 2 + tests/api/test_ossl_ec.c | 90 ++++++++++++++++++++++++ tests/api/test_ossl_ec.h | 2 + wolfcrypt/src/ecc.c | 46 +++++++++--- wolfcrypt/src/port/nxp/ksdk_port.c | 15 ++-- wolfcrypt/src/port/st/stm32.c | 9 ++- 9 files changed, 313 insertions(+), 20 deletions(-) diff --git a/doc/dox_comments/header_files/ecc.h b/doc/dox_comments/header_files/ecc.h index 589918da90f..e237a57eee4 100644 --- a/doc/dox_comments/header_files/ecc.h +++ b/doc/dox_comments/header_files/ecc.h @@ -1027,6 +1027,7 @@ int wc_ecc_point_is_at_infinity(ecc_point *p); \return MP_OKAY Returns on successful operation. \return MP_INIT_E Returned if there is an error initializing an integer for use with the multiple precision integer (mp_int) library. + \return ECC_OUT_OF_RANGE_E Returned if k is negative or too large. \param k The multiplicand. \param G Base point to multiply. diff --git a/src/pk_ec.c b/src/pk_ec.c index 91da8ed6fb5..2b3c215ef69 100644 --- a/src/pk_ec.c +++ b/src/pk_ec.c @@ -2484,6 +2484,57 @@ static int wolfssl_ec_point_mul(int curveIdx, ecc_point* r, mp_int* n, return ret; } +/* Reduce a scalar that is negative or wider than the prime modulo the number + * of points on the curve, as OpenSSL does. + * + * @param [in] group EC group. + * @param [in, out] s Scalar. May point to NULL. Set to *r when reduced. + * @param [out] r New BN holding the reduced scalar. + * @param [in, out] card Order times cofactor. Computed when first needed. + * @return 1 on success. + * @return 0 on error. + */ +static int ec_scalar_nnmod(const WOLFSSL_EC_GROUP* group, + const WOLFSSL_BIGNUM** s, WOLFSSL_BIGNUM** r, WOLFSSL_BIGNUM** card) +{ + int ret = 1; + + if ((*s == NULL) || ((!wolfSSL_BN_is_negative(*s)) && + (wolfSSL_BN_num_bits(*s) <= wolfSSL_EC_GROUP_get_degree(group)))) { + return 1; + } + + if (*card == NULL) { + *card = wolfSSL_BN_new(); + if ((*card == NULL) || + (wolfSSL_EC_GROUP_get_order(group, *card, NULL) != 1) || + (wolfSSL_BN_mul_word(*card, + (WOLFSSL_BN_ULONG)ecc_sets[group->curve_idx].cofactor) != + 1)) { + WOLFSSL_MSG("Getting the number of points failed"); + ret = 0; + } + } + if (ret == 1) { + *r = wolfSSL_BN_new(); + if ((*r == NULL) || (wolfSSL_BN_mod(*r, *s, *card, NULL) != 1)) { + WOLFSSL_MSG("wolfSSL_BN_mod error"); + ret = 0; + } + } + /* The remainder keeps the sign of s with some math libraries. */ + if ((ret == 1) && wolfSSL_BN_is_negative(*r) && + (wolfSSL_BN_add(*r, *r, *card) != 1)) { + WOLFSSL_MSG("wolfSSL_BN_add error"); + ret = 0; + } + if (ret == 1) { + *s = *r; + } + + return ret; +} + /* Sum the scalar multiplications of the base point and n, and q and m. * * r = base point * n + q * m @@ -2504,6 +2555,9 @@ int wolfSSL_EC_POINT_mul(const WOLFSSL_EC_GROUP *group, WOLFSSL_EC_POINT *r, WOLFSSL_BN_CTX *ctx) { int ret = 1; + WOLFSSL_BIGNUM* nr = NULL; + WOLFSSL_BIGNUM* mr = NULL; + WOLFSSL_BIGNUM* card = NULL; /* No BN operations performed. */ (void)ctx; @@ -2542,6 +2596,13 @@ int wolfSSL_EC_POINT_mul(const WOLFSSL_EC_GROUP *group, WOLFSSL_EC_POINT *r, } #endif + if (ret == 1) { + ret = ec_scalar_nnmod(group, &n, &nr, &card); + } + if (ret == 1) { + ret = ec_scalar_nnmod(group, &m, &mr, &card); + } + if (ret == 1) { mp_int* ni = (n != NULL) ? (mp_int*)n->internal : NULL; ecc_point* qi = (q != NULL) ? (ecc_point*)q->internal : NULL; @@ -2568,6 +2629,9 @@ int wolfSSL_EC_POINT_mul(const WOLFSSL_EC_GROUP *group, WOLFSSL_EC_POINT *r, } #endif + wolfSSL_BN_free(card); + wolfSSL_BN_clear_free(mr); + wolfSSL_BN_clear_free(nr); return ret; } #endif /* !WOLFSSL_ATECC508A && !WOLFSSL_ATECC608A && !HAVE_SELFTEST && diff --git a/tests/api/test_ecc.c b/tests/api/test_ecc.c index b3936ec0545..649c192757e 100644 --- a/tests/api/test_ecc.c +++ b/tests/api/test_ecc.c @@ -4170,6 +4170,110 @@ int test_wc_ecc_mulmod_zero(void) return EXPECT_RESULT(); } /* END test_wc_ecc_mulmod_zero */ +#if defined(HAVE_ECC) && defined(WOLFSSL_PUBLIC_MP) && \ + defined(WOLFSSL_PUBLIC_ECC_ADD_DBL) && \ + !defined(NO_ECC256) && !defined(NO_ECC_SECP) && \ + (!defined(ECC_MIN_KEY_SZ) || (ECC_MIN_KEY_SZ <= 256)) && \ + !(defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \ + defined(WOLFSSL_MICROCHIP_TA100) || \ + defined(WOLFSSL_VALIDATE_ECC_IMPORT)) && \ + !defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(HAVE_SELFTEST) && \ + !defined(WOLFSSL_KCAPI_ECC) && !defined(WOLFSSL_STM32_PKA) && \ + !defined(FREESCALE_LTC_ECC) && \ + !defined(HAVE_FIPS) && \ + !defined(WOLFSSL_NO_ECC_SW) && (defined(USE_FAST_MATH) || \ + defined(USE_INTEGER_HEAP_MATH) || defined(WOLFSSL_SP_INT_NEGATIVE)) +#define TEST_ECC_MULMOD_NEGATIVE +#endif + +/* + * Testing that scalar multiplication rejects a negative or oversized scalar. + */ +int test_wc_ecc_mulmod_negative(void) +{ + EXPECT_DECLS; +#ifdef TEST_ECC_MULMOD_NEGATIVE + const ecc_set_type* dp = NULL; + ecc_key base; + ecc_key curve; + ecc_point* r = NULL; + mp_int neg; + mp_int five; + mp_int order; + mp_int wide; + + XMEMSET(&base, 0, sizeof(ecc_key)); + XMEMSET(&curve, 0, sizeof(ecc_key)); + XMEMSET(&neg, 0, sizeof(mp_int)); + XMEMSET(&five, 0, sizeof(mp_int)); + XMEMSET(&order, 0, sizeof(mp_int)); + XMEMSET(&wide, 0, sizeof(mp_int)); + + ExpectNotNull(dp = wc_ecc_get_curve_params( + wc_ecc_get_curve_idx(ECC_SECP256R1))); + ExpectIntEQ(wc_ecc_init(&base), 0); + ExpectIntEQ(wc_ecc_init(&curve), 0); + ExpectIntEQ(mp_init_multi(&neg, &five, &order, &wide, NULL, NULL), 0); + ExpectIntEQ(mp_read_radix(&neg, "-5", MP_RADIX_HEX), 0); + /* 2^258 */ + ExpectIntEQ(mp_read_radix(&wide, "4" + "0000000000000000000000000000000000000000000000000000000000000000", + MP_RADIX_HEX), 0); + ExpectIntEQ(mp_set(&five, 5), 0); + ExpectNotNull(r = wc_ecc_new_point()); + + if (dp != NULL) { + ExpectIntEQ(wc_ecc_import_raw_ex(&base, dp->Gx, dp->Gy, NULL, + ECC_SECP256R1), 0); + ExpectIntEQ(wc_ecc_import_raw_ex(&curve, dp->Af, dp->prime, NULL, + ECC_SECP256R1), 0); + ExpectIntEQ(mp_read_radix(&order, dp->order, MP_RADIX_HEX), 0); + } + ExpectTrue(mp_isneg(&neg)); + + ExpectIntEQ(wc_ecc_mulmod(&neg, &base.pubkey, r, curve.pubkey.x, + curve.pubkey.y, 1), WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + ExpectIntEQ(wc_ecc_mulmod_ex(&neg, &base.pubkey, r, curve.pubkey.x, + curve.pubkey.y, 0, NULL), WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + ExpectIntEQ(wc_ecc_mulmod_ex2(&neg, &base.pubkey, r, curve.pubkey.x, + curve.pubkey.y, &order, NULL, 1, NULL), + WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + ExpectIntEQ(wc_ecc_mulmod(&wide, &base.pubkey, r, curve.pubkey.x, + curve.pubkey.y, 1), WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + ExpectIntEQ(wc_ecc_mulmod_ex2(&wide, &base.pubkey, r, curve.pubkey.x, + curve.pubkey.y, &order, NULL, 1, NULL), + WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); +#if defined(ECC_SHAMIR) && !defined(WOLFSSL_SP_MATH) + ExpectIntEQ(ecc_mul2add(&base.pubkey, &neg, &base.pubkey, &five, r, + curve.pubkey.x, curve.pubkey.y, NULL), + WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + ExpectIntEQ(ecc_mul2add(&base.pubkey, &five, &base.pubkey, &neg, r, + curve.pubkey.x, curve.pubkey.y, NULL), + WC_NO_ERR_TRACE(ECC_OUT_OF_RANGE_E)); + ExpectIntEQ(ecc_mul2add(&base.pubkey, NULL, &base.pubkey, &five, r, + curve.pubkey.x, curve.pubkey.y, NULL), + WC_NO_ERR_TRACE(ECC_BAD_ARG_E)); + ExpectIntEQ(ecc_mul2add(&base.pubkey, &five, &base.pubkey, &five, r, + curve.pubkey.x, curve.pubkey.y, NULL), 0); +#endif + /* Positive scalars are still accepted. */ + ExpectIntEQ(wc_ecc_mulmod(&five, &base.pubkey, r, curve.pubkey.x, + curve.pubkey.y, 1), 0); + + wc_ecc_del_point(r); + mp_free(&wide); + mp_free(&order); + mp_free(&five); + mp_free(&neg); + wc_ecc_free(&curve); + wc_ecc_free(&base); +#ifdef FP_ECC + wc_ecc_fp_free(); +#endif +#endif + return EXPECT_RESULT(); +} /* END test_wc_ecc_mulmod_negative */ + /* * Testing that wc_ecc_mulmod() of the private key and the base point gives * the public key, on every curve compiled in. This covers curves the SP code diff --git a/tests/api/test_ecc.h b/tests/api/test_ecc.h index 17b463b2720..c12ec9e4337 100644 --- a/tests/api/test_ecc.h +++ b/tests/api/test_ecc.h @@ -75,6 +75,7 @@ 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_zero(void); +int test_wc_ecc_mulmod_negative(void); int test_wc_ecc_mulmod_curves(void); int test_wc_ecc_is_valid_idx(void); int test_wc_ecc_get_curve_id_from_oid(void); @@ -140,6 +141,7 @@ int test_wc_ecc_make_key_id_dev(void); 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_zero), \ + TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_negative), \ TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_curves), \ TEST_DECL_GROUP("ecc", test_wc_ecc_is_valid_idx), \ TEST_DECL_GROUP("ecc", test_wc_ecc_get_curve_id_from_oid), \ diff --git a/tests/api/test_ossl_ec.c b/tests/api/test_ossl_ec.c index 823276ad8cd..5a9e4363a45 100644 --- a/tests/api/test_ossl_ec.c +++ b/tests/api/test_ossl_ec.c @@ -897,6 +897,96 @@ int test_wolfSSL_EC_POINT(void) return EXPECT_RESULT(); } +/* EC_POINT_mul() reduces a negative or over-wide scalar modulo the order, as + * OpenSSL does. */ +int test_wolfSSL_EC_POINT_mul_negative(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_EXTRA) && !defined(WOLFSSL_SP_MATH) && \ + (!defined(WOLFSSL_SP_MATH_ALL) || defined(WOLFSSL_SP_INT_NEGATIVE)) && \ + !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \ + !defined(WOLFSSL_MICROCHIP_TA100) && !defined(HAVE_SELFTEST) && \ + !defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(WOLFSSL_NO_ECC_SW) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)) && !defined(NO_ECC256) && \ + !defined(NO_ECC_SECP) && \ + (!defined(ECC_MIN_KEY_SZ) || (ECC_MIN_KEY_SZ <= 256)) + EC_GROUP* group = NULL; + EC_POINT* g = NULL; + EC_POINT* r1 = NULL; + EC_POINT* r2 = NULL; + BIGNUM* order = NULL; + BIGNUM* neg = NULL; + BIGNUM* pos = NULL; + + ExpectNotNull(group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)); + ExpectNotNull(g = EC_POINT_new(group)); + ExpectNotNull(r1 = EC_POINT_new(group)); + ExpectNotNull(r2 = EC_POINT_new(group)); + ExpectNotNull(order = BN_new()); + ExpectNotNull(neg = BN_new()); + ExpectNotNull(pos = BN_new()); + ExpectIntEQ(EC_GROUP_get_order(group, order, NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, g, BN_value_one(), NULL, NULL, NULL), 1); + + /* -5 and order - 5 */ + ExpectIntGT(BN_hex2bn(&neg, "-5"), 0); + ExpectIntEQ(BN_is_negative(neg), 1); + ExpectIntEQ(BN_add(pos, order, neg), 1); + + ExpectIntEQ(EC_POINT_mul(group, r1, neg, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, r2, pos, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + + ExpectIntEQ(EC_POINT_mul(group, r1, NULL, g, neg, NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, r2, NULL, g, pos, NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + + ExpectIntEQ(EC_POINT_mul(group, r1, neg, g, BN_value_one(), NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, r2, pos, g, BN_value_one(), NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + + ExpectIntEQ(EC_POINT_mul(group, r1, BN_value_one(), g, neg, NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + + /* The caller's scalar is left as it was. */ + ExpectIntEQ(BN_is_negative(neg), 1); + + /* -order is 0 modulo the order. */ + BN_zero(neg); + ExpectIntEQ(BN_sub(neg, neg, order), 1); + ExpectIntEQ(EC_POINT_mul(group, r1, neg, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_is_at_infinity(group, r1), 1); + + /* order * 2^64 + 5 is 5 modulo the order. */ + ExpectIntEQ(BN_lshift(pos, order, 64), 1); + ExpectIntEQ(BN_add_word(pos, 5), 1); + ExpectIntEQ(BN_set_word(neg, 5), 1); + ExpectIntEQ(EC_POINT_mul(group, r1, pos, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, r2, neg, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + ExpectIntEQ(EC_POINT_mul(group, r1, NULL, g, pos, NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + + /* 2^256 + 5 is one bit wider than the prime. */ + ExpectIntEQ(BN_set_word(pos, 1), 1); + ExpectIntEQ(BN_lshift(pos, pos, 256), 1); + ExpectIntEQ(BN_add_word(pos, 5), 1); + ExpectIntEQ(BN_mod(neg, pos, order, NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, r1, pos, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_mul(group, r2, neg, NULL, NULL, NULL), 1); + ExpectIntEQ(EC_POINT_cmp(group, r1, r2, NULL), 0); + + BN_free(pos); + BN_free(neg); + BN_free(order); + EC_POINT_free(r2); + EC_POINT_free(r1); + EC_POINT_free(g); + EC_GROUP_free(group); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_SPAKE(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_ec.h b/tests/api/test_ossl_ec.h index 52fc422caa1..2e06e0a5e33 100644 --- a/tests/api/test_ossl_ec.h +++ b/tests/api/test_ossl_ec.h @@ -30,6 +30,7 @@ int test_wolfSSL_EC_GROUP(void); int test_wolfSSL_PEM_read_bio_ECPKParameters(void); int test_wolfSSL_i2d_ECPKParameters(void); int test_wolfSSL_EC_POINT(void); +int test_wolfSSL_EC_POINT_mul_negative(void); int test_wolfSSL_SPAKE(void); int test_wolfSSL_EC_KEY_generate(void); int test_EC_i2d(void); @@ -54,6 +55,7 @@ int test_wolfSSL_EC_POINT_point2hex_oversized(void); TEST_DECL_GROUP("ossl_ec", test_wolfSSL_PEM_read_bio_ECPKParameters), \ TEST_DECL_GROUP("ossl_ec", test_wolfSSL_i2d_ECPKParameters), \ TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_POINT), \ + TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_POINT_mul_negative), \ TEST_DECL_GROUP("ossl_ec", test_wolfSSL_SPAKE), \ TEST_DECL_GROUP("ossl_ec", test_wolfSSL_EC_KEY_generate), \ TEST_DECL_GROUP("ossl_ec", test_EC_i2d), \ diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 312753f8008..0abbea33934 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3978,8 +3978,8 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a, goto exit; } - /* k can't have more bits than modulus count plus 1 */ - if (mp_count_bits(k) > mp_count_bits(modulus) + 1) { + /* 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)) { err = ECC_OUT_OF_RANGE_E; goto exit; } @@ -4082,7 +4082,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_count_bits(k) > mp_count_bits(modulus)) { + if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus))) { return ECC_OUT_OF_RANGE_E; } if (mp_count_bits(G->x) > mp_count_bits(modulus) || @@ -4204,6 +4204,9 @@ 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_YES) { + return ECC_OUT_OF_RANGE_E; + } #ifdef HAVE_ECC_CDH if (mp_count_bits(modulus) > mp_count_bits(order)) { @@ -4307,6 +4310,9 @@ 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))) { + return ECC_OUT_OF_RANGE_E; + } if (mp_count_bits(G->x) > mp_count_bits(modulus) || mp_count_bits(G->y) > mp_count_bits(modulus) || mp_count_bits(G->z) > mp_count_bits(modulus)) { @@ -9163,6 +9169,9 @@ int ecc_mul2add(ecc_point* A, mp_int* kA, modulus == NULL) { return ECC_BAD_ARG_E; } + if ((mp_isneg(kA) == MP_YES) || (mp_isneg(kB) == MP_YES)) { + return ECC_OUT_OF_RANGE_E; + } #ifndef WOLFSSL_NO_MALLOC /* allocate memory */ @@ -14730,12 +14739,23 @@ int ecc_mul2add(ecc_point* A, mp_int* kA, int idx1 = -1, idx2 = -1, err, mpInit = 0; mp_digit mp = 0; #ifdef WOLFSSL_SMALL_STACK - mp_int *mu = (mp_int *)XMALLOC(sizeof *mu, NULL, DYNAMIC_TYPE_ECC_BUFFER); + mp_int *mu = NULL; +#else + mp_int mu[1]; +#endif + + if ((A == NULL) || (kA == NULL) || (B == NULL) || (kB == NULL) || + (C == NULL) || (modulus == NULL)) { + return ECC_BAD_ARG_E; + } + if ((mp_isneg(kA) == MP_YES) || (mp_isneg(kB) == MP_YES)) { + return ECC_OUT_OF_RANGE_E; + } +#ifdef WOLFSSL_SMALL_STACK + mu = (mp_int *)XMALLOC(sizeof *mu, NULL, DYNAMIC_TYPE_ECC_BUFFER); if (mu == NULL) return MP_MEM; -#else - mp_int mu[1]; #endif err = mp_init(mu); @@ -14879,8 +14899,8 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a, return ECC_BAD_ARG_E; } - /* k can't have more bits than modulus count plus 1 */ - if (mp_count_bits(k) > mp_count_bits(modulus) + 1) { + /* 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)) { return ECC_OUT_OF_RANGE_E; } @@ -14978,6 +14998,9 @@ 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))) { + return ECC_OUT_OF_RANGE_E; + } if (mp_count_bits(G->x) > mp_count_bits(modulus) || mp_count_bits(G->y) > mp_count_bits(modulus) || mp_count_bits(G->z) > mp_count_bits(modulus)) { @@ -15043,8 +15066,8 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a, return ECC_BAD_ARG_E; } - /* k can't have more bits than order */ - if (mp_count_bits(k) > mp_count_bits(order)) { + /* k can't be negative or have more bits than order */ + if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(order))) { return ECC_OUT_OF_RANGE_E; } @@ -15145,6 +15168,9 @@ 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))) { + return ECC_OUT_OF_RANGE_E; + } if (mp_count_bits(G->x) > mp_count_bits(modulus) || mp_count_bits(G->y) > mp_count_bits(modulus) || mp_count_bits(G->z) > mp_count_bits(modulus)) { diff --git a/wolfcrypt/src/port/nxp/ksdk_port.c b/wolfcrypt/src/port/nxp/ksdk_port.c index d8fe888cd18..f3966775b43 100644 --- a/wolfcrypt/src/port/nxp/ksdk_port.c +++ b/wolfcrypt/src/port/nxp/ksdk_port.c @@ -1006,12 +1006,20 @@ 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 || modulus == NULL) { return ECC_BAD_ARG_E; } + if (mp_isneg(k) || (mp_count_bits(k) > mp_count_bits(modulus) + 1)) { + return ECC_OUT_OF_RANGE_E; + } + if ((mp_count_bits(G->x) > mp_count_bits(modulus)) || + (mp_count_bits(G->y) > mp_count_bits(modulus))) { + return IS_POINT_E; + } szModulus = mp_unsigned_bin_size(modulus); szkbin = mp_unsigned_bin_size(k); /* make sure LTC big number variable is large enough */ - if (szModulus > LTC_MAX_INT_BYTES / 2) { + if ((szModulus > LTC_MAX_INT_BYTES / 2) || + (szModulus > (int)sizeof(Gxbin))) { return MP_MEM; } @@ -1046,12 +1054,9 @@ int wc_ecc_mulmod_ex(const mp_int *k, ecc_point *G, ecc_point *R, mp_int* a, res = mp_read_unsigned_bin(R->x, Gxbin, size); if (res == MP_OKAY) { res = mp_read_unsigned_bin(R->y, Gybin, size); - /* if k is negative, we compute the multiplication with abs(-k) - * with result (x, y) and modify the result to (x, -y) - */ #if (!defined(WOLFSSL_SP_MATH) && !defined(WOLFSSL_SP_MATH_ALL)) || \ defined(WOLFSSL_SP_INT_NEGATIVE) - R->y->sign = k->sign; + R->y->sign = MP_ZPOS; #endif } if (res == MP_OKAY) diff --git a/wolfcrypt/src/port/st/stm32.c b/wolfcrypt/src/port/st/stm32.c index 597e4cdfe9e..673d7180f49 100644 --- a/wolfcrypt/src/port/st/stm32.c +++ b/wolfcrypt/src/port/st/stm32.c @@ -6405,6 +6405,9 @@ 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))) { + return ECC_OUT_OF_RANGE_E; + } szModulus = mp_unsigned_bin_size(modulus); @@ -6466,14 +6469,10 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a, res = mp_read_unsigned_bin(R->x, Gxbin, szModulus); if (res == MP_OKAY) { res = mp_read_unsigned_bin(R->y, Gybin, szModulus); - #if defined(USE_FAST_MATH) || defined(USE_INTEGER_HEAP_MATH) || \ ((defined(WOLFSSL_SP_MATH) || defined(WOLFSSL_SP_MATH_ALL)) && \ defined(WOLFSSL_SP_INT_NEGATIVE)) - /* if k is negative, we compute the multiplication with abs(-k) - * with result (x, y) and modify the result to (x, -y) - */ - R->y->sign = k->sign; + R->y->sign = MP_ZPOS; #endif } if (res == MP_OKAY)