refactor: introduce `_ecmult_gen_ge` helper (preventing accidental gej leaks)
What the developer wrote
Authored by Sebastian Falbesoner
refactor: introduce `_ecmult_gen_ge` helper (preventing accidental gej leaks)
Scalar multiplication with the generator point frequently involves a
conversion to affine coordinates and clearing out the temporary Jacobian
group element object after to avoid leaking secret key material, i.e.
executing the following three steps:
- secp256k1_ecmult_gen(ctx, &rj, ...)
- secp256k1_ge_set_gej(&r, &rj)
- secp256k1_gej_clear(&rj)
This commit introduces a corresponding helper to deduplicate code
and mitigate the risk that last step is forgotten (which can easily
happen and is not detected by tests).
The idea came up during a conversation with furszy, see
https://github.com/bitcoin-core/secp256k1/pull/1765#issuecomment-4482838033
What changed, and why it matters
This is a defensive code cleanup in a Bitcoin cryptography library. It introduces a helper function that wraps a common three-step pattern when multiplying by the generator point, ensuring that temporary sensitive Jacobian coordinate data is always wiped from memory. The change reduces the chance that a future developer forgets to clear that temporary data, which could theoretically leak tiny fragments of secret key information through memory side channels. It does not fix a known active bug or reported vulnerability.
Treat as a low-risk hardening improvement. No urgent action required, but downstream users should include it in routine updates. Reviewers should verify that all call sites previously clearing gej now use the helper, and that no new code paths reintroduce manual ecmult_gen + ge_set_gej without clearing.
Security signals we found
New helper ensures intermediate Jacobian coordinates are always cleared after generator multiplication
Commit message states goal is to prevent accidental gej leaks of secret scalar information
Refactors existing call sites in ECDSA sign, Schnorr sign, pubkey creation, and blinding setup
No functional algorithm change; purely defensive hardening against developer error
Evidence from the diff
The commit refactors scalar multiplication with the generator point (secp256k1_ecmult_gen) by adding secp256k1_ecmult_gen_ge, which performs ecmult_gen, converts the result to affine coordinates, and then explicitly clears the intermediate Jacobian group element. Previously, callers had to remember to call secp256k1_gej_clear themselves. The new helper is used in ECDSA signing, Schnorr signing, public key creation, and ecmult_gen blinding setup. The commit message explicitly frames this as preventing accidental gej leaks, noting that such leaks are not detected by tests.
Changed components
src/ecdsa_impl.hsrc/ecmult_gen.hsrc/ecmult_gen_impl.hsrc/modules/schnorrsig/main_impl.hsrc/secp256k1.cInspect captured patch +14 / −16
diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h
index 163539e..32f1e58 100644
--- a/src/ecdsa_impl.h
+++ b/src/ecdsa_impl.h
@@ -273,14 +273,12 @@ static int secp256k1_ecdsa_sig_verify(const secp256k1_scalar *sigr, const secp25
static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, secp256k1_scalar *sigr, secp256k1_scalar *sigs, const secp256k1_scalar *seckey, const secp256k1_scalar *message, const secp256k1_scalar *nonce, int *recid) {
unsigned char b[32];
- secp256k1_gej rp;
secp256k1_ge r;
secp256k1_scalar n;
int overflow = 0;
int high;
- secp256k1_ecmult_gen(ctx, &rp, nonce);
- secp256k1_ge_set_gej(&r, &rp);
+ secp256k1_ecmult_gen_ge(ctx, &r, nonce);
secp256k1_fe_normalize(&r.x);
secp256k1_fe_normalize(&r.y);
secp256k1_fe_get_b32(b, &r.x);
@@ -296,7 +294,6 @@ static int secp256k1_ecdsa_sig_sign(const secp256k1_ecmult_gen_context *ctx, sec
secp256k1_scalar_inverse(sigs, nonce);
secp256k1_scalar_mul(sigs, sigs, &n);
secp256k1_scalar_clear(&n);
- secp256k1_gej_clear(&rp);
secp256k1_ge_clear(&r);
high = secp256k1_scalar_is_high(sigs);
secp256k1_scalar_cond_negate(sigs, high);
diff --git a/src/ecmult_gen.h b/src/ecmult_gen.h
index 8bc4f14..b842e78 100644
--- a/src/ecmult_gen.h
+++ b/src/ecmult_gen.h
@@ -138,6 +138,7 @@ static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context* ctx
/** Multiply with the generator: R = a*G */
static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context* ctx, secp256k1_gej *r, const secp256k1_scalar *a);
+static void secp256k1_ecmult_gen_ge(const secp256k1_ecmult_gen_context* ctx, secp256k1_ge *r, const secp256k1_scalar *a);
static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_hash_ctx *hash_ctx, const unsigned char *seed32);
diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h
index 5a95497..53dc5f3 100644
--- a/src/ecmult_gen_impl.h
+++ b/src/ecmult_gen_impl.h
@@ -281,11 +281,19 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
secp256k1_memclear_explicit(&recoded, sizeof(recoded));
}
+SECP256K1_INLINE static void secp256k1_ecmult_gen_ge(const secp256k1_ecmult_gen_context *ctx, secp256k1_ge *r, const secp256k1_scalar *a) {
+ secp256k1_gej rj;
+ secp256k1_ecmult_gen(ctx, &rj, a);
+ secp256k1_ge_set_gej(r, &rj);
+ /* Jacobian coordinates resulting from our multiplication algorithm could potentially leak
+ * information about the secret input scalar, so clear the memory out to be on the safe side. */
+ secp256k1_gej_clear(&rj);
+}
+
/* Setup blinding values for secp256k1_ecmult_gen. */
static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_hash_ctx *hash_ctx, const unsigned char *seed32) {
secp256k1_scalar b;
secp256k1_scalar diff;
- secp256k1_gej gb;
secp256k1_fe f;
unsigned char nonce32[32];
secp256k1_rfc6979_hmac_sha256 rng;
@@ -325,15 +333,13 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
* which secp256k1_gej_add_ge cannot handle. */
secp256k1_scalar_cmov(&b, &secp256k1_scalar_one, secp256k1_scalar_is_zero(&b));
secp256k1_rfc6979_hmac_sha256_finalize(&rng);
- secp256k1_ecmult_gen(ctx, &gb, &b);
+ secp256k1_ecmult_gen_ge(ctx, &ctx->ge_offset, &b);
secp256k1_scalar_negate(&b, &b);
secp256k1_scalar_add(&ctx->scalar_offset, &b, &diff);
- secp256k1_ge_set_gej(&ctx->ge_offset, &gb);
/* Clean up. */
secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
secp256k1_scalar_clear(&b);
- secp256k1_gej_clear(&gb);
secp256k1_fe_clear(&f);
secp256k1_rfc6979_hmac_sha256_clear(&rng);
}
diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h
index 5100557..efc7216 100644
--- a/src/modules/schnorrsig/main_impl.h
+++ b/src/modules/schnorrsig/main_impl.h
@@ -123,7 +123,6 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
secp256k1_scalar sk;
secp256k1_scalar e;
secp256k1_scalar k;
- secp256k1_gej rj;
secp256k1_ge pk;
secp256k1_ge r;
unsigned char nonce32[32] = { 0 };
@@ -160,8 +159,7 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
ret &= !secp256k1_scalar_is_zero(&k);
secp256k1_scalar_cmov(&k, &secp256k1_scalar_one, !ret);
- secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &rj, &k);
- secp256k1_ge_set_gej(&r, &rj);
+ secp256k1_ecmult_gen_ge(&ctx->ecmult_gen_ctx, &r, &k);
/* We declassify r to allow using it as a branch point. This is fine
* because r is not a secret. */
@@ -183,7 +181,6 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
secp256k1_scalar_clear(&sk);
secp256k1_memclear_explicit(seckey, sizeof(seckey));
secp256k1_memclear_explicit(nonce32, sizeof(nonce32));
- secp256k1_gej_clear(&rj);
return ret;
}
diff --git a/src/secp256k1.c b/src/secp256k1.c
index e4b80ff..b216872 100644
--- a/src/secp256k1.c
+++ b/src/secp256k1.c
@@ -624,15 +624,12 @@ int secp256k1_ec_seckey_verify(const secp256k1_context* ctx, const unsigned char
}
static int secp256k1_ec_pubkey_create_helper(const secp256k1_ecmult_gen_context *ecmult_gen_ctx, secp256k1_scalar *seckey_scalar, secp256k1_ge *p, const unsigned char *seckey) {
- secp256k1_gej pj;
int ret;
ret = secp256k1_scalar_set_b32_seckey(seckey_scalar, seckey);
secp256k1_scalar_cmov(seckey_scalar, &secp256k1_scalar_one, !ret);
- secp256k1_ecmult_gen(ecmult_gen_ctx, &pj, seckey_scalar);
- secp256k1_ge_set_gej(p, &pj);
- secp256k1_gej_clear(&pj);
+ secp256k1_ecmult_gen_ge(ecmult_gen_ctx, p, seckey_scalar);
return ret;
}
Why this scored 36/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.