diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index b943fe2a..2159eed5 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -277,8 +277,8 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 /* Cleanup. */ secp256k1_fe_clear(&neg); secp256k1_ge_clear(&add); - secp256k1_memclear(&adds, sizeof(adds)); - secp256k1_memclear(&recoded, sizeof(recoded)); + secp256k1_memclear_explicit(&adds, sizeof(adds)); + secp256k1_memclear_explicit(&recoded, sizeof(recoded)); } /* Setup blinding values for secp256k1_ecmult_gen. */ @@ -310,7 +310,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const VERIFY_CHECK(seed32 != NULL); memcpy(keydata + 32, seed32, 32); secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64); - secp256k1_memclear(keydata, sizeof(keydata)); + secp256k1_memclear_explicit(keydata, sizeof(keydata)); /* Compute projective blinding factor (cannot be 0). */ secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); @@ -331,7 +331,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_ge_set_gej(&ctx->ge_offset, &gb); /* Clean up. */ - secp256k1_memclear(nonce32, sizeof(nonce32)); + secp256k1_memclear_explicit(nonce32, sizeof(nonce32)); secp256k1_scalar_clear(&b); secp256k1_gej_clear(&gb); secp256k1_fe_clear(&f); diff --git a/src/field_impl.h b/src/field_impl.h index 896507a3..7aa7de43 100644 --- a/src/field_impl.h +++ b/src/field_impl.h @@ -19,7 +19,7 @@ #endif SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { - secp256k1_memclear(a, sizeof(secp256k1_fe)); + secp256k1_memclear_explicit(a, sizeof(secp256k1_fe)); } SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { diff --git a/src/group_impl.h b/src/group_impl.h index b8f2395d..81a24b9d 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -337,11 +337,11 @@ static void secp256k1_ge_set_infinity(secp256k1_ge *r) { } static void secp256k1_gej_clear(secp256k1_gej *r) { - secp256k1_memclear(r, sizeof(secp256k1_gej)); + secp256k1_memclear_explicit(r, sizeof(secp256k1_gej)); } static void secp256k1_ge_clear(secp256k1_ge *r) { - secp256k1_memclear(r, sizeof(secp256k1_ge)); + secp256k1_memclear_explicit(r, sizeof(secp256k1_ge)); } static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd) { diff --git a/src/hash_impl.h b/src/hash_impl.h index 1065acd6..43419177 100644 --- a/src/hash_impl.h +++ b/src/hash_impl.h @@ -172,7 +172,7 @@ static void secp256k1_sha256_initialize_tagged(secp256k1_sha256 *hash, const uns } static void secp256k1_sha256_clear(secp256k1_sha256 *hash) { - secp256k1_memclear(hash, sizeof(*hash)); + secp256k1_memclear_explicit(hash, sizeof(*hash)); } static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t keylen) { @@ -200,7 +200,7 @@ static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const rkey[n] ^= 0x5c ^ 0x36; } secp256k1_sha256_write(&hash->inner, rkey, sizeof(rkey)); - secp256k1_memclear(rkey, sizeof(rkey)); + secp256k1_memclear_explicit(rkey, sizeof(rkey)); } static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size) { @@ -211,12 +211,12 @@ static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned unsigned char temp[32]; secp256k1_sha256_finalize(&hash->inner, temp); secp256k1_sha256_write(&hash->outer, temp, 32); - secp256k1_memclear(temp, sizeof(temp)); + secp256k1_memclear_explicit(temp, sizeof(temp)); secp256k1_sha256_finalize(&hash->outer, out32); } static void secp256k1_hmac_sha256_clear(secp256k1_hmac_sha256 *hash) { - secp256k1_memclear(hash, sizeof(*hash)); + secp256k1_memclear_explicit(hash, sizeof(*hash)); } static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen) { @@ -285,7 +285,7 @@ static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 } static void secp256k1_rfc6979_hmac_sha256_clear(secp256k1_rfc6979_hmac_sha256 *rng) { - secp256k1_memclear(rng, sizeof(*rng)); + secp256k1_memclear_explicit(rng, sizeof(*rng)); } #undef Round diff --git a/src/modules/ecdh/main_impl.h b/src/modules/ecdh/main_impl.h index 842b5359..9f2dfdd5 100644 --- a/src/modules/ecdh/main_impl.h +++ b/src/modules/ecdh/main_impl.h @@ -62,8 +62,8 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se ret = hashfp(output, x, y, data); - secp256k1_memclear(x, sizeof(x)); - secp256k1_memclear(y, sizeof(y)); + secp256k1_memclear_explicit(x, sizeof(x)); + secp256k1_memclear_explicit(y, sizeof(y)); secp256k1_scalar_clear(&s); secp256k1_ge_clear(&pt); secp256k1_gej_clear(&res); diff --git a/src/modules/ellswift/main_impl.h b/src/modules/ellswift/main_impl.h index 745a9691..0d13f734 100644 --- a/src/modules/ellswift/main_impl.h +++ b/src/modules/ellswift/main_impl.h @@ -582,7 +582,7 @@ int secp256k1_ellswift_xdh(const secp256k1_context *ctx, unsigned char *output, /* Invoke hasher */ ret = hashfp(output, sx, ell_a64, ell_b64, data); - secp256k1_memclear(sx, sizeof(sx)); + secp256k1_memclear_explicit(sx, sizeof(sx)); secp256k1_fe_clear(&px); secp256k1_scalar_clear(&s); diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index d8dcd00c..2c8778b3 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -385,10 +385,10 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c secp256k1_scalar_set_b32(&k[i], buf, NULL); /* Attempt to erase secret data */ - secp256k1_memclear(buf, sizeof(buf)); + secp256k1_memclear_explicit(buf, sizeof(buf)); secp256k1_sha256_clear(&sha_tmp); } - secp256k1_memclear(rand, sizeof(rand)); + secp256k1_memclear_explicit(rand, sizeof(rand)); secp256k1_sha256_clear(&sha); } @@ -518,7 +518,7 @@ int secp256k1_musig_nonce_gen_counter(const secp256k1_context* ctx, secp256k1_mu if (!secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, &pubkey, msg32, keyagg_cache, extra_input32)) { return 0; } - secp256k1_memclear(seckey, sizeof(seckey)); + secp256k1_memclear_explicit(seckey, sizeof(seckey)); return 1; } @@ -679,7 +679,7 @@ int secp256k1_musig_partial_sign(const secp256k1_context* ctx, secp256k1_musig_p ret = secp256k1_musig_secnonce_load(ctx, k, &pk, secnonce); /* Set nonce to zero to avoid nonce reuse. This will cause subsequent calls * of this function to fail */ - memset(secnonce, 0, sizeof(*secnonce)); + secp256k1_memzero_explicit(secnonce, sizeof(*secnonce)); if (!ret) { secp256k1_musig_partial_sign_clear(&sk, k); return 0; diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 13d92449..b410b19e 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -94,7 +94,7 @@ static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *ms secp256k1_sha256_write(&sha, msg, msglen); secp256k1_sha256_finalize(&sha, nonce32); secp256k1_sha256_clear(&sha); - secp256k1_memclear(masked_key, sizeof(masked_key)); + secp256k1_memclear_explicit(masked_key, sizeof(masked_key)); return 1; } @@ -190,8 +190,8 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi secp256k1_memczero(sig64, 64, !ret); secp256k1_scalar_clear(&k); secp256k1_scalar_clear(&sk); - secp256k1_memclear(seckey, sizeof(seckey)); - secp256k1_memclear(nonce32, sizeof(nonce32)); + secp256k1_memclear_explicit(seckey, sizeof(seckey)); + secp256k1_memclear_explicit(nonce32, sizeof(nonce32)); secp256k1_gej_clear(&rj); return ret; diff --git a/src/scalar_impl.h b/src/scalar_impl.h index 0232a8c2..9965c2ba 100644 --- a/src/scalar_impl.h +++ b/src/scalar_impl.h @@ -28,7 +28,7 @@ static const secp256k1_scalar secp256k1_scalar_one = SECP256K1_SCALAR_CONST(0, 0 static const secp256k1_scalar secp256k1_scalar_zero = SECP256K1_SCALAR_CONST(0, 0, 0, 0, 0, 0, 0, 0); SECP256K1_INLINE static void secp256k1_scalar_clear(secp256k1_scalar *r) { - secp256k1_memclear(r, sizeof(secp256k1_scalar)); + secp256k1_memclear_explicit(r, sizeof(secp256k1_scalar)); } static int secp256k1_scalar_set_b32_seckey(secp256k1_scalar *r, const unsigned char *bin) { diff --git a/src/secp256k1.c b/src/secp256k1.c index 0915af77..26336a45 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -499,7 +499,7 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m } secp256k1_rfc6979_hmac_sha256_finalize(&rng); - secp256k1_memclear(keydata, sizeof(keydata)); + secp256k1_memclear_explicit(keydata, sizeof(keydata)); secp256k1_rfc6979_hmac_sha256_clear(&rng); return 1; } @@ -550,7 +550,7 @@ static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_sc * seckey. As a result is_sec_valid is included in ret only after ret was * used as a branching variable. */ ret &= is_sec_valid; - secp256k1_memclear(nonce32, sizeof(nonce32)); + secp256k1_memclear_explicit(nonce32, sizeof(nonce32)); secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); diff --git a/src/util.h b/src/util.h index 5f29f407..94e0837d 100644 --- a/src/util.h +++ b/src/util.h @@ -219,8 +219,8 @@ static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) { } } -/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. */ -static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) { +/* Zeroes memory to prevent leaking sensitive info. Won't be optimized out. */ +static SECP256K1_INLINE void secp256k1_memzero_explicit(void *ptr, size_t len) { #if defined(_MSC_VER) /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ SecureZeroMemory(ptr, len); @@ -242,6 +242,19 @@ static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) { void *(*volatile const volatile_memset)(void *, int, size_t) = memset; volatile_memset(ptr, 0, len); #endif +} + +/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. + * The state of the memory after this call is unspecified so callers must not + * make any assumptions about its contents. + * + * In VERIFY builds, it has the side effect of marking the memory as undefined. + * This helps to detect use-after-clear bugs where code incorrectly reads from + * cleansed memory during testing. + */ +static SECP256K1_INLINE void secp256k1_memclear_explicit(void *ptr, size_t len) { + /* The current implementation zeroes, but callers must not rely on this */ + secp256k1_memzero_explicit(ptr, len); #ifdef VERIFY SECP256K1_CHECKMEM_UNDEFINE(ptr, len); #endif @@ -277,7 +290,7 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size } ret = (acc == 0); /* acc may contain secret values. Try to explicitly clear it. */ - secp256k1_memclear(&acc, sizeof(acc)); + secp256k1_memclear_explicit(&acc, sizeof(acc)); return ret; }