What the developer wrote
Authored by Lőrinc
musig: check infinity test setup
`pubnonce_summing_to_inf` initializes both sums to infinity before loading the nonce objects.
A load failure could therefore satisfy the expected infinity checks without validating the constructed pair.
Require both nonce-loading operations to succeed before testing their outputs.
What changed, and why it matters
This change fixes a small but real flaw in the MuSig test code. Previously, two test helper functions called internal loading/summing routines and then checked whether the results were the special 'point at infinity' value. Because the input buffers were already initialized to infinity, a failed load or sum could still make the infinity checks pass, so the test would not actually detect a broken implementation. The patch adds explicit success checks so the test fails if those routines return an error. This only affects test code, not the cryptographic library used by real Bitcoin transactions, so it does not create a direct security vulnerability in production software.
No urgent action required for production deployments; the change is in test code only. Developers should ensure the test suite passes with the new assertions and consider auditing other MuSig tests for similarly ignored return values.
Security signals we found
Ignored return value from internal cryptographic routine in test code
Vacuously passing assertion due to pre-initialized infinity values
MuSig nonce aggregation test coverage gap
Evidence from the diff
In src/modules/musig/tests_impl.h, pubnonce_summing_to_inf() and musig_api_tests() previously ignored the return values of secp256k1_musig_sum_pubnonces() and secp256k1_musig_aggnonce_load(). Both functions can return 0 on failure. The destination gej/ge arrays were stack-allocated/zeroed, so on failure they remained at the point-at-infinity representation, causing the subsequent secp256k1_gej_is_infinity/secp256k1_ge_is_infinity checks to pass vacuously. The patch wraps both calls in CHECK(... == 1), ensuring the test aborts if the routines fail. This is a test-hardening fix; no production code path is changed.
Changed components
src/modules/musig/tests_impl.hMuSig module test helperspubnonce_summing_to_inf()musig_api_tests()Inspect captured patch +2 / −2
diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h
index 3a30c23..7b54afd 100644
--- a/src/modules/musig/tests_impl.h
+++ b/src/modules/musig/tests_impl.h
@@ -104,7 +104,7 @@ static void pubnonce_summing_to_inf(secp256k1_musig_pubnonce *pubnonce) {
secp256k1_ge_neg(&ge[1], &ge[1]);
}
- secp256k1_musig_sum_pubnonces(CTX, summed_pubnonces, pubnonce_ptr, 2);
+ CHECK(secp256k1_musig_sum_pubnonces(CTX, summed_pubnonces, pubnonce_ptr, 2) == 1);
CHECK(secp256k1_gej_is_infinity(&summed_pubnonces[0]));
CHECK(secp256k1_gej_is_infinity(&summed_pubnonces[1]));
}
@@ -372,7 +372,7 @@ static void musig_api_tests(void) {
{
/* Check that the aggnonce encodes two points at infinity */
secp256k1_ge aggnonce_pt[2];
- secp256k1_musig_aggnonce_load(CTX, aggnonce_pt, &aggnonce);
+ CHECK(secp256k1_musig_aggnonce_load(CTX, aggnonce_pt, &aggnonce) == 1);
for (i = 0; i < 2; i++) {
CHECK(secp256k1_ge_is_infinity(&aggnonce_pt[i]) == 1);
}
Why this scored 17/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.