Skip to content

Reject projective points in SP math only FP_ECC wc_ecc_mulmod_ex() #11971

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
LinuxJedi wants to merge 3 commits into
base: master
Choose a base branch
from
+153 −4
Open

Reject projective points in SP math only FP_ECC wc_ecc_mulmod_ex() #11971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f57686
Reject projective points in SP math only FP_ECC wc_ecc_mulmod_ex()
LinuxJedi Nov 3, 2026
3fc4589
Skip curves below ECC_MIN_KEY_SZ in test_wc_ecc_mulmod_projective
LinuxJedi Nov 3, 2026
becf581
Drop the z bit count before the affine check in SP math FP_ECC mulmod
LinuxJedi Nov 6, 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
139 changes: 139 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 @@ -4316,6 +4316,145 @@ int test_wc_ecc_mulmod_curves(void)
return EXPECT_RESULT();
} /* END test_wc_ecc_mulmod_curves */

/*
* Testing wc_ecc_mulmod() and wc_ecc_mulmod_ex2() with a base point that is
* not affine.
*
* With FP_ECC, the SP code caches a table for a point keyed on its x and y
* ordinates only. (Gx, Gy, p - 1) is -G but has the same x and y as G, so with
* SP math only a table built from one was used for the other. That build now
* rejects a point whose z is not 1; other builds return the right point.
*/
int test_wc_ecc_mulmod_projective(void)
{
EXPECT_DECLS;
#if defined(HAVE_ECC) && defined(WOLFSSL_PUBLIC_MP) && \
!defined(NO_ECC_SECP) && \
!(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)
static const int curves[] = {
#if (!defined(NO_ECC256) || defined(HAVE_ALL_CURVES)) && \
(ECC_MIN_KEY_SZ <= 256) && \
(!defined(WOLFSSL_SP_MATH) || !defined(WOLFSSL_SP_NO_256))
ECC_SECP256R1,
#endif
#if (defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES)) && \
(ECC_MIN_KEY_SZ <= 384) && \
(!defined(WOLFSSL_SP_MATH) || defined(WOLFSSL_SP_384))
ECC_SECP384R1,
#endif
#if (defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)) && \
(ECC_MIN_KEY_SZ <= 521) && \
(!defined(WOLFSSL_SP_MATH) || defined(WOLFSSL_SP_521))
ECC_SECP521R1,
#endif
ECC_CURVE_INVALID
};
#if defined(WOLFSSL_SP_MATH) && defined(FP_ECC)
const int projRet = WC_NO_ERR_TRACE(ECC_BAD_ARG_E);
#else
const int projRet = 0;
#endif
const ecc_set_type* dp = NULL;
ecc_point* g = NULL;
ecc_point* r = NULL;
ecc_point* kG = NULL;
ecc_point* negKG = NULL;
mp_int k;
mp_int a;
mp_int prime;
mp_int order;
int c;
int i;
int cnt = 1;

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

ExpectIntEQ(mp_init_multi(&k, &a, &prime, &order, NULL, NULL), 0);
ExpectIntEQ(mp_read_radix(&k,
"1A2B3C4D5E6F708192A3B4C5D6E7F8091A2B3C4D5E6F708192A3B4C5D6E7F80",
MP_RADIX_HEX), 0);
ExpectNotNull(g = wc_ecc_new_point());
ExpectNotNull(r = wc_ecc_new_point());
ExpectNotNull(kG = wc_ecc_new_point());
ExpectNotNull(negKG = wc_ecc_new_point());

#ifdef WOLFSSL_PUBLIC_ECC_ADD_DBL
/* Also test wc_ecc_mulmod_ex2(). */
cnt = 2;
#endif

for (c = 0; curves[c] != ECC_CURVE_INVALID; c++) {
ExpectNotNull(dp = wc_ecc_get_curve_params(
wc_ecc_get_curve_idx(curves[c])));
if (dp != 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(&order, dp->order, 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);

/* kG from the affine base point, and -kG. */
ExpectIntEQ(wc_ecc_mulmod(&k, g, kG, &a, &prime, 1), 0);
ExpectIntEQ(wc_ecc_copy_point(kG, negKG), 0);
ExpectIntEQ(mp_sub(&prime, kG->y, negKG->y), 0);

for (i = 0; i < cnt; i++) {
/* (Gx, Gy, p - 1) is -G. */
ExpectIntEQ(mp_sub_d(&prime, 1, g->z), 0);
#ifdef FP_ECC
/* Don't let a table in the generic cache hide the SP code. */
wc_ecc_fp_free();
#endif
if (i == 0) {
ExpectIntEQ(wc_ecc_mulmod(&k, g, r, &a, &prime, 1), projRet);
}
#ifdef WOLFSSL_PUBLIC_ECC_ADD_DBL
else {
ExpectIntEQ(wc_ecc_mulmod_ex2(&k, g, r, &a, &prime, &order,
NULL, 1, NULL), projRet);
}
#endif
#if !defined(WOLFSSL_SP_MATH) || !defined(FP_ECC)
ExpectIntEQ(wc_ecc_cmp_point(r, negKG), MP_EQ);
#endif

/* G again: a table of -G must not be used. */
ExpectIntEQ(mp_set(g->z, 1), 0);
#ifdef FP_ECC
wc_ecc_fp_free();
#endif
ExpectIntEQ(wc_ecc_mulmod(&k, g, r, &a, &prime, 1), 0);
ExpectIntEQ(wc_ecc_cmp_point(r, kG), MP_EQ);
}
}

wc_ecc_del_point(negKG);
wc_ecc_del_point(kG);
wc_ecc_del_point(r);
wc_ecc_del_point(g);
mp_free(&order);
mp_free(&prime);
mp_free(&a);
mp_free(&k);

#ifdef FP_ECC
wc_ecc_fp_free();
#endif
#endif
return EXPECT_RESULT();
} /* END test_wc_ecc_mulmod_projective */

/*
* Testing wc_ecc_is_valid_idx()
*/
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 @@ -76,6 +76,7 @@ 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_curves(void);
int test_wc_ecc_mulmod_projective(void);
int test_wc_ecc_is_valid_idx(void);
int test_wc_ecc_get_curve_id_from_oid(void);
int test_wc_ecc_sig_size_calc(void);
Expand Down Expand Up @@ -141,6 +142,7 @@ int test_wc_ecc_make_key_id_dev(void);
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_curves), \
TEST_DECL_GROUP("ecc", test_wc_ecc_mulmod_projective), \
TEST_DECL_GROUP("ecc", test_wc_ecc_is_valid_idx), \
TEST_DECL_GROUP("ecc", test_wc_ecc_get_curve_id_from_oid), \
TEST_DECL_GROUP("ecc", test_wc_ecc_sig_size_calc), \
Expand Down
16 changes: 12 additions & 4 deletions wolfcrypt/src/ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14979,10 +14979,15 @@ int wc_ecc_mulmod_ex(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
return ECC_BAD_ARG_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)) {
mp_count_bits(G->y) > mp_count_bits(modulus)) {
return IS_POINT_E;
}
/* The SP code caches tables for points keyed on x and y only, so a point
* with z other than 1 could be multiplied using the table of another
* point. Only affine points are supported. */
if (!mp_isone(G->z)) {

@SparkiDev SparkiDev Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Counting bits of G->z above.

@LinuxJedi LinuxJedi Nov 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed the G->z bit count, as z != 1 is rejected right after it (bf24470).

return ECC_BAD_ARG_E;
}

#if defined(WOLFSSL_SM2) && defined(WOLFSSL_SP_SM2)
if ((mp_count_bits(modulus) == 256) && (!mp_is_bit_set(modulus, 224))) {
Expand Down Expand Up @@ -15146,10 +15151,13 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point *G, ecc_point *R, mp_int* a,
return ECC_BAD_ARG_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)) {
mp_count_bits(G->y) > mp_count_bits(modulus)) {
return IS_POINT_E;
}
/* Only affine points are supported - see wc_ecc_mulmod_ex(). */
if (!mp_isone(G->z)) {

@SparkiDev SparkiDev Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Counting bits of G->z above.

@LinuxJedi LinuxJedi Nov 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed the G->z bit count, as z != 1 is rejected right after it (bf24470).

return ECC_BAD_ARG_E;
}

#if defined(WOLFSSL_SM2) && defined(WOLFSSL_SP_SM2)
if ((mp_count_bits(modulus) == 256) && (!mp_is_bit_set(modulus, 224))) {
Expand Down
Loading