Skip to content

Commit b11340b

Browse files
Merge #1849: musig: always clear out secret key in secp256k1_musig_nonce_gen_counter
8479eaf musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter` (Sebastian Falbesoner) Pull request description: Even though `secp256k1_musig_nonce_gen_internal` can currently only fail if the surrounding API function is misused (invalid `keypair` or `keyagg_cache` parameters, making the corresponding [seckey validation](https://github.com/bitcoin-core/secp256k1/blob/c1a9e4fe6471478dba9c5ef61105b9c5213c6bf1/src/modules/musig/session_impl.h#L391) or [pubkey](https://github.com/bitcoin-core/secp256k1/blob/c1a9e4fe6471478dba9c5ef61105b9c5213c6bf1/src/modules/musig/session_impl.h#L404)/[keyaggcache](https://github.com/bitcoin-core/secp256k1/blob/c1a9e4fe6471478dba9c5ef61105b9c5213c6bf1/src/modules/musig/session_impl.h#L397) load calls fail), clearing out the stack memory holding the secret key as well in this case seems reasonable to follow best practices. The issue was reported off-band by l0rinc (thanks!), in the course of analyzing the secp repository with AI tooling. ACKs for top commit: furszy: ACK 8479eaf real-or-random: utACK 8479eaf Tree-SHA512: dc15ed7518c6cd0b1b86d2e0382c546374e94a1c1fa15639ba3db27e083ce53a24ddf4d3cd3328b4dc229258d8cbb0e01f4b63f025d04254845f2bf20cfa5289
2 parents c1a9e4f + 8479eaf commit b11340b

1 file changed

Lines changed: 2 additions & 4 deletions

File tree

src/modules/musig/session_impl.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -483,11 +483,9 @@ int secp256k1_musig_nonce_gen_counter(const secp256k1_context* ctx, secp256k1_mu
483483
(void) ret;
484484
#endif
485485

486-
if (!secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, &pubkey, msg32, keyagg_cache, extra_input32)) {
487-
return 0;
488-
}
486+
ret = secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, &pubkey, msg32, keyagg_cache, extra_input32);
489487
secp256k1_memclear_explicit(seckey, sizeof(seckey));
490-
return 1;
488+
return ret;
491489
}
492490

493491
static int secp256k1_musig_sum_pubnonces(const secp256k1_context* ctx, secp256k1_gej *summed_pubnonces, const secp256k1_musig_pubnonce * const* pubnonces, size_t n_pubnonces) {

0 commit comments

Comments
 (0)