Skip to content

Commit 9bb368d

Browse files
real-or-randomisle2983
authored andcommitted
Use secp256k1_memclear() to clear stack memory instead of memset()
All of the invocations of secp256k1_memclear() operate on stack memory and happen after the function is done with the memory object. This commit replaces existing memset() invocations and also adds secp256k1_memclear() to code locations where clearing was missing; there is no guarantee that this commit covers all code locations where clearing is necessary. Co-Authored-By: isle2983 <isle2983@yahoo.com>
1 parent e3497bb commit 9bb368d

File tree

8 files changed

+23
-21
lines changed

8 files changed

+23
-21
lines changed

src/ecmult_gen_impl.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,8 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
277277
/* Cleanup. */
278278
secp256k1_fe_clear(&neg);
279279
secp256k1_ge_clear(&add);
280-
memset(&adds, 0, sizeof(adds));
281-
memset(&recoded, 0, sizeof(recoded));
280+
secp256k1_memclear(&adds, sizeof(adds));
281+
secp256k1_memclear(&recoded, sizeof(recoded));
282282
}
283283

284284
/* Setup blinding values for secp256k1_ecmult_gen. */
@@ -310,7 +310,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
310310
VERIFY_CHECK(seed32 != NULL);
311311
memcpy(keydata + 32, seed32, 32);
312312
secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64);
313-
memset(keydata, 0, sizeof(keydata));
313+
secp256k1_memclear(keydata, sizeof(keydata));
314314

315315
/* Compute projective blinding factor (cannot be 0). */
316316
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
@@ -325,13 +325,13 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
325325
* which secp256k1_gej_add_ge cannot handle. */
326326
secp256k1_scalar_cmov(&b, &secp256k1_scalar_one, secp256k1_scalar_is_zero(&b));
327327
secp256k1_rfc6979_hmac_sha256_finalize(&rng);
328-
memset(nonce32, 0, 32);
329328
secp256k1_ecmult_gen(ctx, &gb, &b);
330329
secp256k1_scalar_negate(&b, &b);
331330
secp256k1_scalar_add(&ctx->scalar_offset, &b, &diff);
332331
secp256k1_ge_set_gej(&ctx->ge_offset, &gb);
333332

334333
/* Clean up. */
334+
secp256k1_memclear(nonce32, sizeof(nonce32));
335335
secp256k1_scalar_clear(&b);
336336
secp256k1_gej_clear(&gb);
337337
secp256k1_fe_clear(&f);

src/hash_impl.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out
156156
secp256k1_write_be32(&out32[4*i], hash->s[i]);
157157
hash->s[i] = 0;
158158
}
159+
160+
secp256k1_memclear(sizedesc, sizeof(sizedesc));
161+
secp256k1_memclear(hash, sizeof(secp256k1_sha256));
159162
}
160163

161164
/* Initializes a sha256 struct and writes the 64 byte string
@@ -196,7 +199,7 @@ static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const
196199
rkey[n] ^= 0x5c ^ 0x36;
197200
}
198201
secp256k1_sha256_write(&hash->inner, rkey, sizeof(rkey));
199-
memset(rkey, 0, sizeof(rkey));
202+
secp256k1_memclear(rkey, sizeof(rkey));
200203
}
201204

202205
static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size) {
@@ -207,7 +210,7 @@ static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned
207210
unsigned char temp[32];
208211
secp256k1_sha256_finalize(&hash->inner, temp);
209212
secp256k1_sha256_write(&hash->outer, temp, 32);
210-
memset(temp, 0, 32);
213+
secp256k1_memclear(temp, sizeof(temp));
211214
secp256k1_sha256_finalize(&hash->outer, out32);
212215
}
213216

@@ -274,9 +277,7 @@ static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256
274277
}
275278

276279
static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 *rng) {
277-
memset(rng->k, 0, 32);
278-
memset(rng->v, 0, 32);
279-
rng->retry = 0;
280+
secp256k1_memclear(rng, sizeof(secp256k1_rfc6979_hmac_sha256));
280281
}
281282

282283
#undef Round

src/modules/ecdh/main_impl.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,10 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se
6161

6262
ret = hashfp(output, x, y, data);
6363

64-
memset(x, 0, 32);
65-
memset(y, 0, 32);
64+
secp256k1_memclear(x, sizeof(x));
65+
secp256k1_memclear(y, sizeof(y));
6666
secp256k1_scalar_clear(&s);
67+
secp256k1_ge_clear(&pt);
6768

6869
return !!ret & !overflow;
6970
}

src/modules/ellswift/main_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ int secp256k1_ellswift_xdh(const secp256k1_context *ctx, unsigned char *output,
580580
/* Invoke hasher */
581581
ret = hashfp(output, sx, ell_a64, ell_b64, data);
582582

583-
memset(sx, 0, 32);
583+
secp256k1_memclear(sx, sizeof(sx));
584584
secp256k1_fe_clear(&px);
585585
secp256k1_scalar_clear(&s);
586586

src/modules/musig/session_impl.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -385,11 +385,11 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c
385385
secp256k1_scalar_set_b32(&k[i], buf, NULL);
386386

387387
/* Attempt to erase secret data */
388-
memset(buf, 0, sizeof(buf));
389-
memset(&sha_tmp, 0, sizeof(sha_tmp));
388+
secp256k1_memclear(buf, sizeof(buf));
389+
secp256k1_memclear(&sha_tmp, sizeof(sha_tmp));
390390
}
391-
memset(rand, 0, sizeof(rand));
392-
memset(&sha, 0, sizeof(sha));
391+
secp256k1_memclear(rand, sizeof(rand));
392+
secp256k1_memclear(&sha, sizeof(sha));
393393
}
394394

395395
int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *input_nonce, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
@@ -509,7 +509,7 @@ int secp256k1_musig_nonce_gen_counter(const secp256k1_context* ctx, secp256k1_mu
509509
if (!secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, &pubkey, msg32, keyagg_cache, extra_input32)) {
510510
return 0;
511511
}
512-
memset(seckey, 0, sizeof(seckey));
512+
secp256k1_memclear(seckey, sizeof(seckey));
513513
return 1;
514514
}
515515

src/modules/schnorrsig/main_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
187187
secp256k1_memczero(sig64, 64, !ret);
188188
secp256k1_scalar_clear(&k);
189189
secp256k1_scalar_clear(&sk);
190-
memset(seckey, 0, sizeof(seckey));
190+
secp256k1_memclear(seckey, sizeof(seckey));
191191

192192
return ret;
193193
}

src/secp256k1.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m
494494
buffer_append(keydata, &offset, algo16, 16);
495495
}
496496
secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, offset);
497-
memset(keydata, 0, sizeof(keydata));
497+
secp256k1_memclear(keydata, sizeof(keydata));
498498
for (i = 0; i <= counter; i++) {
499499
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
500500
}
@@ -548,7 +548,7 @@ static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_sc
548548
* seckey. As a result is_sec_valid is included in ret only after ret was
549549
* used as a branching variable. */
550550
ret &= is_sec_valid;
551-
memset(nonce32, 0, 32);
551+
secp256k1_memclear(nonce32, sizeof(nonce32));
552552
secp256k1_scalar_clear(&msg);
553553
secp256k1_scalar_clear(&non);
554554
secp256k1_scalar_clear(&sec);

src/util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size
285285
}
286286
ret = (acc == 0);
287287
/* acc may contain secret values. Try to explicitly clear it. */
288-
acc = 0;
288+
secp256k1_memclear(&acc, sizeof(acc));
289289
return ret;
290290
}
291291

0 commit comments

Comments
 (0)