From 34fa8e0b2e9c83b2d6f793238f390b5c198731e6 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Tue, 1 Sep 2026 23:36:54 +0200 Subject: [PATCH] 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 --- src/modules/frost/keygen_impl.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/modules/frost/keygen_impl.h b/src/modules/frost/keygen_impl.h index 65954dd1..f83a8ebc 100644 --- a/src/modules/frost/keygen_impl.h +++ b/src/modules/frost/keygen_impl.h @@ -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);