frost: zero the trusted dealer outputs when key generation fails
secp256k1_frost_trusted_dealer_keygen clears secshares32, thresh_pk and pubshares up front, under a comment promising that "the outputs are unusable if this function fails". The per-participant loop then fills them in one participant at a time, and every failure after that point leaves the shares written so far in the caller's buffer while returning 0. The reachable failure is the zero-share check inside the loop, which has negligible probability, so this is hygiene rather than a live leak. It still contradicts the stated contract, and the buffer it leaves populated holds real secret shares for participants 0..i-1 of a setup the caller has been told to discard. Re-zero all three outputs on the failure path. secshares32 goes through secp256k1_memzero_explicit rather than memset because it is secret and the buffer is dead afterwards. n_participants is validated before any path that can reach the cleanup label, so the lengths are safe to use there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -293,6 +293,14 @@ int secp256k1_frost_trusted_dealer_keygen(const secp256k1_context *ctx, unsigned
|
||||
ret = 1;
|
||||
|
||||
cleanup:
|
||||
if (!ret) {
|
||||
/* The loop above may have written real secret shares for the first
|
||||
* few participants before failing. Zero the outputs again so that a
|
||||
* failed call leaves nothing usable behind, as promised above. */
|
||||
secp256k1_memzero_explicit(secshares32, n_participants * 32);
|
||||
memset(thresh_pk, 0, sizeof(*thresh_pk));
|
||||
memset(pubshares, 0, n_participants * sizeof(*pubshares));
|
||||
}
|
||||
secp256k1_scalar_clear(&secret);
|
||||
secp256k1_scalar_clear(&share);
|
||||
secp256k1_scalar_clear(&x);
|
||||
|
||||
Reference in New Issue
Block a user