ellswift: don't declassify or leave sk in sha256 buffer
What the developer wrote
Authored by furszy
ellswift: don't declassify or leave sk in sha256 buffer
secp256k1_ellswift_create assumes sha256 clears the data in its buffer
after hashing it, which is not the case. So we shouldn't declassify the
whole struct, only the hash result. We should also clear it at the end,
so the sk doesn't linger on the stack when no aux rnd is given.
What changed, and why it matters
This commit fixes a small but real privacy bug in the ElligatorSwift key-encoding helper. The code had wrongly assumed that the SHA-256 hashing routine wipes its internal buffer after feeding data in, so it marked the whole hash state as safe-to-leak and did not clear it. Because the buffer still held a copy of the secret key, that secret key could remain in stack memory longer than intended, especially when no extra random data (auxrnd) was provided. The patch narrows the 'declassify' to only the actual hash output/state and explicitly clears the SHA-256 buffer at the end, removing the leftover secret key from the stack.
Treat as a low-to-moderate security hardening fix. Backport to stable branches that include the ElligatorSwift module, and verify that any downstream code using this module is rebuilt. No immediate incident response is required unless the target threat model includes local stack-memory attacks against this specific API.
Security signals we found
Secret key material retained in SHA-256 internal buffer after write
Over-broad declassification of struct containing secret key copy
Missing stack scrubbing of hash state before function return
Corrective narrowing of declassify to hash state only
Addition of explicit secp256k1_sha256_clear call
Evidence from the diff
In secp256k1_ellswift_create, the implementation feeds seckey32 into a SHA-256 hasher and then called secp256k1_declassify on the entire hash struct, assuming the secret had been consumed and the buffer cleared. secp256k1_sha256_write does not zero the buffer, so a copy of the 32-byte secret key persisted in hash.s. The patch changes the declassify to only hash.s (the running state) and adds secp256k1_sha256_clear(&hash) before returning, ensuring the secret does not linger on the stack. The comment is also corrected from ‘privkey’ to ‘seckey32’. This is a secret-scrubbing / side-channel hygiene fix, not a cryptographic flaw in the ElligatorSwift algorithm itself.
Changed components
src/modules/ellswift/main_impl.hsecp256k1_ellswift_createInspect captured patch +4 / −2
diff --git a/src/modules/ellswift/main_impl.h b/src/modules/ellswift/main_impl.h
index 27cb3db..817e778 100644
--- a/src/modules/ellswift/main_impl.h
+++ b/src/modules/ellswift/main_impl.h
@@ -449,12 +449,13 @@ int secp256k1_ellswift_create(const secp256k1_context *ctx, unsigned char *ell64
secp256k1_fe_normalize_var(&p.x);
secp256k1_fe_normalize_var(&p.y);
- /* Set up hasher state. The used RNG is H(privkey || "\x00"*32 [|| auxrnd32] || cnt++),
+ /* Set up hasher state. The used RNG is H(seckey32 || "\x00"*32 [|| auxrnd32] || cnt++),
* using BIP340 tagged hash with tag "secp256k1_ellswift_create". */
secp256k1_ellswift_sha256_init_create(&hash);
secp256k1_sha256_write(secp256k1_get_hash_context(ctx), &hash, seckey32, 32);
secp256k1_sha256_write(secp256k1_get_hash_context(ctx), &hash, zero32, sizeof(zero32));
- secp256k1_declassify(ctx, &hash, sizeof(hash)); /* private key is hashed now */
+ /* Declassify only hash state. seckey32 has been hashed, but copy remains in the hash buffer */
+ secp256k1_declassify(ctx, &hash.s, sizeof(hash.s));
if (auxrnd32) secp256k1_sha256_write(secp256k1_get_hash_context(ctx), &hash, auxrnd32, 32);
/* Compute ElligatorSwift encoding and construct output. */
@@ -463,6 +464,7 @@ int secp256k1_ellswift_create(const secp256k1_context *ctx, unsigned char *ell64
secp256k1_memczero(ell64, 64, !ret);
secp256k1_scalar_clear(&seckey_scalar);
+ secp256k1_sha256_clear(&hash);
return ret;
}
Why this scored 47/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.