Skip to content

Commit 16f62f5

Browse files
committed
musig: Securely clear secnonce in partial_sign
Replace memset which can be optimized out. Update test to redefine wiped bytes so that the valgrind check on conditioned jumps does not fail.
1 parent f36afb8 commit 16f62f5

File tree

2 files changed

+5
-2
lines changed

2 files changed

+5
-2
lines changed

src/modules/musig/session_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,8 @@ int secp256k1_musig_partial_sign(const secp256k1_context* ctx, secp256k1_musig_p
679679
ret = secp256k1_musig_secnonce_load(ctx, k, &pk, secnonce);
680680
/* Set nonce to zero to avoid nonce reuse. This will cause subsequent calls
681681
* of this function to fail */
682-
memset(secnonce, 0, sizeof(*secnonce));
682+
secp256k1_memclear(secnonce, sizeof(*secnonce));
683+
683684
if (!ret) {
684685
secp256k1_musig_partial_sign_clear(&sk, k);
685686
return 0;

src/modules/musig/tests_impl.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,9 @@ static void musig_api_tests(void) {
400400

401401
memcpy(&secnonce_tmp, &secnonce[0], sizeof(secnonce_tmp));
402402
CHECK(secp256k1_musig_partial_sign(CTX, &partial_sig[0], &secnonce_tmp, &keypair[0], &keyagg_cache, &session) == 1);
403-
/* The secnonce is set to 0 and subsequent signing attempts fail */
403+
/* The secnonce is set to 0 and subsequent signing attempts fail.
404+
* Redefine memory first as secp256k1_musig_partial_sign securely clears and undefines the bytes.*/
405+
SECP256K1_CHECKMEM_DEFINE(&secnonce_tmp, sizeof(secnonce_tmp));
404406
CHECK(secp256k1_memcmp_var(&secnonce_tmp, zeros132, sizeof(secnonce_tmp)) == 0);
405407
CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sign(CTX, &partial_sig[0], &secnonce_tmp, &keypair[0], &keyagg_cache, &session));
406408
memcpy(&secnonce_tmp, &secnonce[0], sizeof(secnonce_tmp));

0 commit comments

Comments
 (0)