Public commit record
68/100 · Adequate
What the developer wrote
Authored by John Moffett
Split memclear into two versions
secp256k1_memclear has the side effect of undefining bytes for
valgrind checks. In some cases, we may want to zero bytes
but allow subsequent reads. So we split memclear into
memclear_explicit, which makes no guarantees about the content
of the buffer on return, and memzero_explicit, which guarantees
zero value on return.
Change the memset in partial_sign to use memzero_explicit.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code-quality and testing-hardening change inside a widely used cryptographic library. It splits one internal memory-wiping helper into two: one that guarantees zeros (used where the code later reads the buffer) and one that intentionally marks memory as undefined in test builds (used where the buffer should never be read again). The only functional change visible in the diff is replacing a plain memset with the guaranteed-zero helper in one MuSig partial-signing path, which makes the code's intent clearer and avoids a potential test-only false positive. There is no direct evidence this fixes an exploitable vulnerability in production.
Recommended action
Treat as a defensive hardening commit. Reviewers should verify that all switched call sites truly do not read the cleared buffer afterward, and that the new memzero_explicit call in MuSig partial signing is not followed by any read of secnonce. No urgent security deployment is indicated by the diff alone, but merging is sensible as part of normal hardening.
Security signals we found
01
Refactoring of secret-memory clearing primitives
02
Use of compiler-barrier / volatile memset to prevent dead-store elimination
03
Addition of VERIFY-build memory-undefined marking for use-after-clear detection
04
Single functional replacement of memset with hardened memzero_explicit in MuSig partial signing nonce wipe
Evidence from the diff
The patch refactors secp256k1_memclear into secp256k1_memzero_explicit (guaranteed zero, not optimized out) and secp256k1_memclear_explicit (state unspecified; in VERIFY builds marks memory undefined via SECP256K1_CHECKMEM_UNDEFINE). Most existing callers are switched to memclear_explicit because they only wipe secrets and never read the buffer again. The single behavioral change is in secp256k1_musig_partial_sign, where memset(secnonce, 0, …) is replaced with secp256k1_memzero_explicit(secnonce, …). That location intentionally zeroes the secret nonce after signing to prevent nonce reuse, and the buffer is not read afterward, so the change is semantically equivalent but uses the library’s hardened, non-optimizable zero primitive. The commit does not change production behavior beyond that; it primarily strengthens valgrind/MSAN-based detection of use-after-clear bugs in debug builds.
Changed components
src/util.hsrc/modules/musig/session_impl.hsrc/ecmult_gen_impl.hsrc/field_impl.hsrc/group_impl.hsrc/hash_impl.hsrc/modules/ecdh/main_impl.hsrc/modules/ellswift/main_impl.hsrc/modules/schnorrsig/main_impl.hsrc/scalar_impl.hsrc/secp256k1.cInspect captured patch +41 / −28
diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h
index b943fe2..2159eed 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 896507a..7aa7de4 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 b8f2395..81a24b9 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 1065acd..4341917 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 842b535..9f2dfdd 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 745a969..0d13f73 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 d8dcd00..2c8778b 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 13d9244..b410b19 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 0232a8c..9965c2b 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 0915af7..26336a4 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 5f29f40..94e0837 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;
}
Risk score
Our methodology →Why this scored 24/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.