ellswift: check test operation results
What the developer wrote
Authored by Lőrinc
ellswift: check test operation results
The EllSwift tests inspected outputs without requiring encoding, decoding, or public-key loading to succeed.
They also discarded the field parser result despite relying on its no-overflow precondition.
Check each deterministic success result before inspecting its output.
What changed, and why it matters
This commit fixes test code for the EllSwift module so that it actually checks whether encoding, decoding, and public-key loading operations succeed before using their outputs. Previously the tests silently ignored failure return values, which could hide bugs during testing but does not change the production library behavior or introduce a direct vulnerability in running software.
No immediate production action is required. Developers should ensure test suites are run after this commit so that any latent EllSwift failures are surfaced rather than masked.
Security signals we found
Missing return-value checks in cryptographic test code
Potential silent test failures in EllSwift encode/decode roundtrip tests
Potential silent test failures in public-key load operations
Potential silent test failures in field-element parsing
Evidence from the diff
The patch adds CHECK(…) assertions around secp256k1_ellswift_encode, secp256k1_ellswift_decode, secp256k1_pubkey_load, and secp256k1_fe_set_b32_limit calls in the EllSwift test implementations. Previously these calls discarded return values, meaning test failures in those APIs would not abort the test suite. The change only affects test files (tests_impl.h and tests_exhaustive_impl.h), not the library implementation, so it is a test-hardening fix rather than a runtime security fix.
Changed components
src/modules/ellswift/tests_impl.hsrc/modules/ellswift/tests_exhaustive_impl.hInspect captured patch +10 / −10
diff --git a/src/modules/ellswift/tests_exhaustive_impl.h b/src/modules/ellswift/tests_exhaustive_impl.h
index 839c24a..2019534 100644
--- a/src/modules/ellswift/tests_exhaustive_impl.h
+++ b/src/modules/ellswift/tests_exhaustive_impl.h
@@ -30,8 +30,8 @@ static void test_exhaustive_ellswift(const secp256k1_context *ctx, const secp256
CHECK(secp256k1_ellswift_create(ctx, ell64, sec32, NULL));
/* Decode ellswift pubkey and check that it matches the precomputed group element. */
- secp256k1_ellswift_decode(ctx, &pub_decoded, ell64);
- secp256k1_pubkey_load(ctx, &ge_decoded, &pub_decoded);
+ CHECK(secp256k1_ellswift_decode(ctx, &pub_decoded, ell64) == 1);
+ CHECK(secp256k1_pubkey_load(ctx, &ge_decoded, &pub_decoded) == 1);
CHECK(secp256k1_ge_eq_var(&ge_decoded, &group[i]));
}
}
diff --git a/src/modules/ellswift/tests_impl.h b/src/modules/ellswift/tests_impl.h
index 7da08d5..71788c5 100644
--- a/src/modules/ellswift/tests_impl.h
+++ b/src/modules/ellswift/tests_impl.h
@@ -249,9 +249,9 @@ void ellswift_encode_decode_roundtrip_tests(void) {
secp256k1_pubkey_save(&pubkey, &g);
testrand256(rnd32);
/* Convert the public key to ElligatorSwift and back. */
- secp256k1_ellswift_encode(CTX, ell64, &pubkey, rnd32);
- secp256k1_ellswift_decode(CTX, &pubkey2, ell64);
- secp256k1_pubkey_load(CTX, &g2, &pubkey2);
+ CHECK(secp256k1_ellswift_encode(CTX, ell64, &pubkey, rnd32) == 1);
+ CHECK(secp256k1_ellswift_decode(CTX, &pubkey2, ell64) == 1);
+ CHECK(secp256k1_pubkey_load(CTX, &g2, &pubkey2) == 1);
/* Compare with original. */
CHECK(secp256k1_ge_eq_var(&g, &g2));
}
@@ -276,8 +276,8 @@ void ellswift_create_tests(void) {
ret = secp256k1_ellswift_create(CTX, ell64, sec32, (i & 1) ? auxrnd32 : NULL);
CHECK(ret);
/* Decode it, and compare with traditionally-computed public key. */
- secp256k1_ellswift_decode(CTX, &pub, ell64);
- secp256k1_pubkey_load(CTX, &dec, &pub);
+ CHECK(secp256k1_ellswift_decode(CTX, &pub, ell64) == 1);
+ CHECK(secp256k1_pubkey_load(CTX, &dec, &pub) == 1);
secp256k1_ecmult(&res, NULL, &secp256k1_scalar_zero, &sec);
CHECK(secp256k1_gej_eq_ge_var(&res, &dec));
}
@@ -300,15 +300,15 @@ void ellswift_compute_shared_secret_tests(void) {
/* Generate random ElligatorSwift encoding for the remote key and decode it. */
testrand256_test(ell64);
testrand256_test(ell64 + 32);
- secp256k1_ellswift_decode(CTX, &pub, ell64);
- secp256k1_pubkey_load(CTX, &dec, &pub);
+ CHECK(secp256k1_ellswift_decode(CTX, &pub, ell64) == 1);
+ CHECK(secp256k1_pubkey_load(CTX, &dec, &pub) == 1);
secp256k1_gej_set_ge(&decj, &dec);
/* Compute the X coordinate of seckey*pubkey using ellswift_xdh. Note that we
* pass ell64 as claimed (but incorrect) encoding for sec32 here; this works
* because the "hasher" function we use here ignores the ell64 arguments. */
ret = secp256k1_ellswift_xdh(CTX, share32, ell64, ell64, sec32, i & 1, &ellswift_xdh_hash_x32, NULL);
CHECK(ret);
- (void)secp256k1_fe_set_b32_limit(&share_x, share32); /* no overflow is possible */
+ CHECK(secp256k1_fe_set_b32_limit(&share_x, share32)); /* no overflow is possible */
SECP256K1_FE_VERIFY(&share_x);
/* Compute seckey*pubkey directly. */
secp256k1_ecmult(&resj, &decj, &sec, NULL);
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.