refactor: introduce _ecmult_gen_ge helper (preventing accidental gej leaks)

src/bench_ecmult.c

@@ -88,7 +88,7 @@ static void bench_ecmult_teardown_helper(bench_data* data, size_t* seckey_offset
             secp256k1_scalar_add(&sum_scalars, &sum_scalars, &s);
         }
     }
-    secp256k1_ecmult_gen(&data->ctx->ecmult_gen_ctx, &tmp, &sum_scalars);
+    secp256k1_ecmult_gen_gej(&data->ctx->ecmult_gen_ctx, &tmp, &sum_scalars);
     CHECK(secp256k1_gej_eq_var(&tmp, &sum_output));
 }
 
@@ -104,7 +104,7 @@ static void bench_ecmult_gen(void* arg, int iters) {
     int i;
 
     for (i = 0; i < iters; ++i) {
-        secp256k1_ecmult_gen(&data->ctx->ecmult_gen_ctx, &data->output[i], &data->scalars[(data->offset1+i) % POINTS]);
+        secp256k1_ecmult_gen_gej(&data->ctx->ecmult_gen_ctx, &data->output[i], &data->scalars[(data->offset1+i) % POINTS]);
     }
 }
 

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);

src/ecmult_gen.h

@@ -137,7 +137,8 @@ static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx
 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_gej(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);
 

src/ecmult_gen_impl.h

@@ -51,7 +51,7 @@ static void secp256k1_ecmult_gen_scalar_diff(secp256k1_scalar* diff) {
     secp256k1_scalar_add(diff, diff, &neghalf);
 }
 
-static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp256k1_gej *r, const secp256k1_scalar *gn) {
+static void secp256k1_ecmult_gen_gej(const secp256k1_ecmult_gen_context *ctx, secp256k1_gej *r, const secp256k1_scalar *gn) {
     uint32_t comb_off;
     secp256k1_ge add;
     secp256k1_fe neg;
@@ -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_gej(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);
 }

src/modules/musig/session_impl.h

@@ -415,7 +415,7 @@ static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp
 
     /* Compute pubnonce as two gejs */
     for (i = 0; i < 2; i++) {
-        secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &nonce_ptj[i], &k[i]);
+        secp256k1_ecmult_gen_gej(&ctx->ecmult_gen_ctx, &nonce_ptj[i], &k[i]);
         secp256k1_scalar_clear(&k[i]);
     }
 

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;
 }

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;
 }
 

src/tests.c

@@ -329,7 +329,7 @@ static void run_proper_context_tests(int use_prealloc) {
     /*** attempt to use them ***/
     testutil_random_scalar_order_test(&msg);
     testutil_random_scalar_order_test(&key);
-    secp256k1_ecmult_gen(&my_ctx->ecmult_gen_ctx, &pubj, &key);
+    secp256k1_ecmult_gen_gej(&my_ctx->ecmult_gen_ctx, &pubj, &key);
     secp256k1_ge_set_gej(&pub, &pubj);
 
     /* obtain a working nonce */
@@ -4311,11 +4311,11 @@ static void test_ec_combine(void) {
         secp256k1_scalar s;
         testutil_random_scalar_order_test(&s);
         secp256k1_scalar_add(&sum, &sum, &s);
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &Qj, &s);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &Qj, &s);
         secp256k1_ge_set_gej(&Q, &Qj);
         secp256k1_pubkey_save(&data[i - 1], &Q);
         d[i - 1] = &data[i - 1];
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &Qj, &sum);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &Qj, &sum);
         secp256k1_ge_set_gej(&Q, &Qj);
         secp256k1_pubkey_save(&sd, &Q);
         CHECK(secp256k1_ec_pubkey_combine(CTX, &sd2, d, i) == 1);
@@ -4593,9 +4593,9 @@ static void test_ecmult_target(const secp256k1_scalar* target, int mode) {
 
     /* EC multiplications */
     if (mode == 0) {
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &p1j, &n1);
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &p2j, &n2);
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &ptj, target);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &p1j, &n1);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &p2j, &n2);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &ptj, target);
     } else if (mode == 1) {
         secp256k1_ecmult(&p1j, &pj, &n1, &secp256k1_scalar_zero);
         secp256k1_ecmult(&p2j, &pj, &n2, &secp256k1_scalar_zero);
@@ -5162,7 +5162,7 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) {
             secp256k1_scalar_mul(&scalars[filled], &sc_tmp, &g_scalar);
             secp256k1_scalar_inverse_var(&sc_tmp, &sc_tmp);
             secp256k1_scalar_negate(&sc_tmp, &sc_tmp);
-            secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &gejs[filled], &sc_tmp);
+            secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &gejs[filled], &sc_tmp);
             ++filled;
             ++mults;
         }
@@ -5642,7 +5642,7 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar
     size_t i;
     secp256k1_gej_set_ge(&gj, &secp256k1_ge_const_g);
     secp256k1_gej_set_infinity(&infj);
-    secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &rj[0], x);
+    secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &rj[0], x);
     secp256k1_ecmult(&rj[1], &gj, x, NULL);
     secp256k1_ecmult(&rj[2], &gj, x, &secp256k1_scalar_zero);
     secp256k1_ecmult(&rj[3], &infj, &secp256k1_scalar_zero, x);
@@ -5796,13 +5796,13 @@ static void test_ecmult_gen_blind(void) {
     secp256k1_ge p;
     secp256k1_ge pge;
     testutil_random_scalar_order_test(&key);
-    secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &pgej, &key);
+    secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &pgej, &key);
     testrand256(seed32);
     b = CTX->ecmult_gen_ctx.scalar_offset;
     p = CTX->ecmult_gen_ctx.ge_offset;
     secp256k1_ecmult_gen_blind(&CTX->ecmult_gen_ctx, secp256k1_get_hash_context(CTX), seed32);
     CHECK(!secp256k1_scalar_eq(&b, &CTX->ecmult_gen_ctx.scalar_offset));
-    secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &pgej2, &key);
+    secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &pgej2, &key);
     CHECK(!gej_xyz_equals_gej(&pgej, &pgej2));
     CHECK(!secp256k1_ge_eq_var(&p, &CTX->ecmult_gen_ctx.ge_offset));
     secp256k1_ge_set_gej(&pge, &pgej);
@@ -5832,7 +5832,7 @@ static void test_ecmult_gen_edge_cases(void) {
 
     for (i = -1; i < 2; ++i) {
         /* Run test with gn = i - scalar_offset (so that the ecmult_gen recoded value represents i). */
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &res1, &gn);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &res1, &gn);
         secp256k1_ecmult(&res2, NULL, &secp256k1_scalar_zero, &gn);
         secp256k1_ecmult_const(&res3, &secp256k1_ge_const_g, &gn);
         CHECK(secp256k1_gej_eq_var(&res1, &res2));
@@ -6524,7 +6524,7 @@ static void test_ecdsa_sign_verify(void) {
     int recid;
     testutil_random_scalar_order_test(&msg);
     testutil_random_scalar_order_test(&key);
-    secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &pubj, &key);
+    secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &pubj, &key);
     secp256k1_ge_set_gej(&pub, &pubj);
     getrec = testrand_bits(1);
     /* The specific way in which this conditional is written sidesteps a potential bug in clang.
@@ -7292,7 +7292,7 @@ static void run_ecdsa_edge_cases(void) {
         secp256k1_scalar_negate(&ss, &ss);
         secp256k1_scalar_inverse(&ss, &ss);
         secp256k1_scalar_set_int(&sr, 1);
-        secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &keyj, &sr);
+        secp256k1_ecmult_gen_gej(&CTX->ecmult_gen_ctx, &keyj, &sr);
         secp256k1_ge_set_gej(&key, &keyj);
         msg = ss;
         CHECK(secp256k1_ecdsa_sig_verify(&sr, &ss, &key, &msg) == 0);

src/tests_exhaustive.c

@@ -425,7 +425,7 @@ int main(int argc, char** argv) {
                 secp256k1_ge generated;
 
                 secp256k1_scalar_set_int(&scalar_i, i);
-                secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &generatedj, &scalar_i);
+                secp256k1_ecmult_gen_gej(&ctx->ecmult_gen_ctx, &generatedj, &scalar_i);
                 secp256k1_ge_set_gej(&generated, &generatedj);
 
                 CHECK(!secp256k1_ge_is_infinity(&group[i]));