ecmult: Use size_t for array indices into tables
What the developer wrote
Authored by Tim Ruffing
ecmult: Use size_t for array indices into tables
What changed, and why it matters
This commit changes several loop counters and a macro from signed 'int' or 'long' types to unsigned 'size_t' when indexing into precomputed elliptic-curve multiplication tables. The main practical effect is to avoid undefined behavior or incorrect results on platforms where the table size calculation could overflow a signed type, and to make the code's intent clearer. There is no direct evidence in the commit that this fixes an exploitable vulnerability in normal Bitcoin Core usage, but it is a defensive correctness improvement in low-level cryptographic code.
Treat as a low-risk hardening/correctness patch. Review whether any supported build configuration or window size could have triggered the signed-overflow behavior, and consider backporting if 32-bit builds or large window sizes are supported. No urgent security response is indicated by the diff alone.
Security signals we found
Signed/unsigned type mismatch in array indexing
Potential signed left-shift overflow in ECMULT_TABLE_SIZE macro
Defensive hardening of low-level elliptic-curve multiplication implementation
No explicit security claim or CVE reference in commit message
Evidence from the diff
The patch converts table-index variables and the ECMULT_TABLE_SIZE macro to size_t. The macro previously used ‘1L << ((w)-2)’, which on 32-bit platforms is a signed 32-bit shift and can overflow/invoke undefined behavior for larger window values, yielding negative or truncated table sizes. By casting to size_t, the shift becomes unsigned and matches the expected array-index semantics. Loop variables in secp256k1_ecmult_compute_table, secp256k1_ecmult_strauss_wnaf, secp256k1_ecmult_pippenger_wnaf, and print_table are also changed from int to size_t to match the unsigned table sizes and prevent signed/unsigned comparison warnings or subtle bugs. The change is defensive and improves portability, especially on 32-bit or exotic platforms.
Changed components
src/ecmult.hsrc/ecmult_compute_table_impl.hsrc/ecmult_impl.hsrc/precompute_ecmult.cInspect captured patch +17 / −16
diff --git a/src/ecmult.h b/src/ecmult.h
index 326a5ee..8d0a9f4 100644
--- a/src/ecmult.h
+++ b/src/ecmult.h
@@ -38,7 +38,7 @@
#endif
/** The number of entries a table with precomputed multiples needs to have. */
-#define ECMULT_TABLE_SIZE(w) (1L << ((w)-2))
+#define ECMULT_TABLE_SIZE(w) ((size_t)1 << ((w)-2))
/** Double multiply: R = na*A + ng*G */
static void secp256k1_ecmult(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_scalar *na, const secp256k1_scalar *ng);
diff --git a/src/ecmult_compute_table_impl.h b/src/ecmult_compute_table_impl.h
index 69d59ce..09b899b 100644
--- a/src/ecmult_compute_table_impl.h
+++ b/src/ecmult_compute_table_impl.h
@@ -16,7 +16,7 @@
static void secp256k1_ecmult_compute_table(secp256k1_ge_storage* table, int window_g, const secp256k1_gej* gen) {
secp256k1_gej gj;
secp256k1_ge ge, dgen;
- int j;
+ size_t j;
gj = *gen;
secp256k1_ge_set_gej_var(&ge, &gj);
diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h
index c421750..4046cd4 100644
--- a/src/ecmult_impl.h
+++ b/src/ecmult_impl.h
@@ -311,8 +311,9 @@ static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state *
}
for (np = 0; np < no; ++np) {
- for (i = 0; i < ECMULT_TABLE_SIZE(WINDOW_A); i++) {
- secp256k1_fe_mul(&state->aux[np * ECMULT_TABLE_SIZE(WINDOW_A) + i], &state->pre_a[np * ECMULT_TABLE_SIZE(WINDOW_A) + i].x, &secp256k1_const_beta);
+ size_t j;
+ for (j = 0; j < ECMULT_TABLE_SIZE(WINDOW_A); j++) {
+ secp256k1_fe_mul(&state->aux[np * ECMULT_TABLE_SIZE(WINDOW_A) + j], &state->pre_a[np * ECMULT_TABLE_SIZE(WINDOW_A) + j].x, &secp256k1_const_beta);
}
}
@@ -517,7 +518,6 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi
size_t np;
size_t no = 0;
int i;
- int j;
for (np = 0; np < num; ++np) {
if (secp256k1_scalar_is_zero(&sc[np]) || secp256k1_ge_is_infinity(&pt[np])) {
@@ -535,16 +535,17 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi
for (i = n_wnaf - 1; i >= 0; i--) {
secp256k1_gej running_sum;
+ int j;
+ size_t buc;
- for(j = 0; j < ECMULT_TABLE_SIZE(bucket_window+2); j++) {
- secp256k1_gej_set_infinity(&buckets[j]);
+ for (buc = 0; buc < ECMULT_TABLE_SIZE(bucket_window+2); buc++) {
+ secp256k1_gej_set_infinity(&buckets[buc]);
}
for (np = 0; np < no; ++np) {
int n = state->wnaf_na[np*n_wnaf + i];
struct secp256k1_pippenger_point_state point_state = state->ps[np];
secp256k1_ge tmp;
- int idx;
if (i == 0) {
/* correct for wnaf skew */
@@ -555,16 +556,16 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi
}
}
if (n > 0) {
- idx = (n - 1)/2;
- secp256k1_gej_add_ge_var(&buckets[idx], &buckets[idx], &pt[point_state.input_pos], NULL);
+ buc = (n - 1)/2;
+ secp256k1_gej_add_ge_var(&buckets[buc], &buckets[buc], &pt[point_state.input_pos], NULL);
} else if (n < 0) {
- idx = -(n + 1)/2;
+ buc = -(n + 1)/2;
secp256k1_ge_neg(&tmp, &pt[point_state.input_pos]);
- secp256k1_gej_add_ge_var(&buckets[idx], &buckets[idx], &tmp, NULL);
+ secp256k1_gej_add_ge_var(&buckets[buc], &buckets[buc], &tmp, NULL);
}
}
- for(j = 0; j < bucket_window; j++) {
+ for (j = 0; j < bucket_window; j++) {
secp256k1_gej_double_var(r, r, NULL);
}
@@ -577,8 +578,8 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi
*
* The doubling is done implicitly by deferring the final window doubling (of 'r').
*/
- for(j = ECMULT_TABLE_SIZE(bucket_window+2) - 1; j > 0; j--) {
- secp256k1_gej_add_var(&running_sum, &running_sum, &buckets[j], NULL);
+ for (buc = ECMULT_TABLE_SIZE(bucket_window+2) - 1; buc > 0; buc--) {
+ secp256k1_gej_add_var(&running_sum, &running_sum, &buckets[buc], NULL);
secp256k1_gej_add_var(r, r, &running_sum, NULL);
}
diff --git a/src/precompute_ecmult.c b/src/precompute_ecmult.c
index 021fe39..8579c85 100644
--- a/src/precompute_ecmult.c
+++ b/src/precompute_ecmult.c
@@ -20,7 +20,7 @@
#include "ecmult_compute_table_impl.h"
static void print_table(FILE *fp, const char *name, int window_g, const secp256k1_ge_storage* table) {
- int j;
+ size_t j;
int i;
fprintf(fp, "const secp256k1_ge_storage %s[ECMULT_TABLE_SIZE(WINDOW_G)] = {\n", name);
Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.