test: Add non-NULL checks for "pointer of array" API functions
What the developer wrote
Authored by Sebastian Falbesoner
test: Add non-NULL checks for "pointer of array" API functions
What changed, and why it matters
This commit only adds new test cases to the project's test suite. It checks that several public-key and signature functions correctly reject arrays that contain a NULL pointer. There are no changes to the actual library code, so this does not fix or introduce a security issue by itself. It is a hardening of the test coverage for existing API behavior.
No security action required. Treat as normal test-coverage improvement. If the tests fail, then the underlying implementation would need review, but the commit itself does not alter behavior.
Security signals we found
NULL-pointer validation tests added for array-of-pointers API inputs
No library implementation code changed
Functions under test already return illegal/invalid for NULL array elements
Evidence from the diff
The diff adds non-NULL element checks in test files (src/tests.c and src/modules/musig/tests_impl.h) for secp256k1_ec_pubkey_combine, secp256k1_ec_pubkey_sort, secp256k1_musig_pubkey_agg, secp256k1_musig_nonce_agg, and secp256k1_musig_partial_sig_agg. Each test temporarily replaces one array element with NULL and asserts CHECK_ILLEGAL. No implementation code is modified. The commit therefore documents/verifies existing defensive behavior rather than changing it.
Changed components
src/tests.csrc/modules/musig/tests_impl.hInspect captured patch +38 / −0
diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h
index b4ba185..de09c5e 100644
--- a/src/modules/musig/tests_impl.h
+++ b/src/modules/musig/tests_impl.h
@@ -201,6 +201,13 @@ static void musig_api_tests(void) {
CHECK(secp256k1_musig_pubkey_agg(CTX, &agg_pk, &keyagg_cache, pk_ptr, 2) == 1);
CHECK(secp256k1_musig_pubkey_agg(CTX, NULL, &keyagg_cache, pk_ptr, 2) == 1);
CHECK(secp256k1_musig_pubkey_agg(CTX, &agg_pk, NULL, pk_ptr, 2) == 1);
+ /* check that NULL in array of public key pointers is not allowed */
+ for (i = 0; i < 2; i++) {
+ const secp256k1_pubkey *original_ptr = pk_ptr[i];
+ pk_ptr[i] = NULL;
+ CHECK_ILLEGAL(CTX, secp256k1_musig_pubkey_agg(CTX, &agg_pk, NULL, pk_ptr, 2));
+ pk_ptr[i] = original_ptr;
+ }
CHECK_ILLEGAL(CTX, secp256k1_musig_pubkey_agg(CTX, &agg_pk, &keyagg_cache, NULL, 2));
CHECK(memcmp_and_randomize(agg_pk.data, zeros132, sizeof(agg_pk.data)) == 0);
CHECK_ILLEGAL(CTX, secp256k1_musig_pubkey_agg(CTX, &agg_pk, &keyagg_cache, invalid_pk_ptr2, 2));
@@ -350,6 +357,13 @@ static void musig_api_tests(void) {
/** Receive nonces and aggregate **/
CHECK(secp256k1_musig_nonce_agg(CTX, &aggnonce, pubnonce_ptr, 2) == 1);
+ /* check that NULL in array of public nonce pointers is not allowed */
+ for (i = 0; i < 2; i++) {
+ const secp256k1_musig_pubnonce *original_ptr = pubnonce_ptr[i];
+ pubnonce_ptr[i] = NULL;
+ CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, &aggnonce, pubnonce_ptr, 2));
+ pubnonce_ptr[i] = original_ptr;
+ }
CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, NULL, pubnonce_ptr, 2));
CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, &aggnonce, NULL, 2));
CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, &aggnonce, pubnonce_ptr, 0));
@@ -474,6 +488,13 @@ static void musig_api_tests(void) {
/** Signature aggregation and verification */
CHECK(secp256k1_musig_partial_sig_agg(CTX, pre_sig, &session, partial_sig_ptr, 2) == 1);
+ /* check that NULL in array of partial signature pointers is not allowed */
+ for (i = 0; i < 2; i++) {
+ const secp256k1_musig_partial_sig *original_ptr = partial_sig_ptr[i];
+ partial_sig_ptr[i] = NULL;
+ CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, pre_sig, &session, partial_sig_ptr, 2));
+ partial_sig_ptr[i] = original_ptr;
+ }
CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, NULL, &session, partial_sig_ptr, 2));
CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, pre_sig, NULL, partial_sig_ptr, 2));
CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, pre_sig, &invalid_session, partial_sig_ptr, 2));
diff --git a/src/tests.c b/src/tests.c
index 4029de5..14aa785 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -6052,6 +6052,7 @@ static void run_eckey_edge_case_test(void) {
secp256k1_pubkey pubkey_negone;
const secp256k1_pubkey *pubkeys[3];
size_t len;
+ int i;
/* Group order is too large, reject. */
CHECK(secp256k1_ec_seckey_verify(CTX, orderc) == 0);
SECP256K1_CHECKMEM_UNDEFINE(&pubkey, sizeof(pubkey));
@@ -6245,6 +6246,14 @@ static void run_eckey_edge_case_test(void) {
CHECK(secp256k1_ec_pubkey_combine(CTX, &pubkey, pubkeys, 3) == 1);
SECP256K1_CHECKMEM_CHECK(&pubkey, sizeof(secp256k1_pubkey));
CHECK(secp256k1_memcmp_var(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
+ /* check that NULL in array of pubkey pointers is not allowed */
+ for (i = 0; i < 3; i++) {
+ const secp256k1_pubkey *original_ptr = pubkeys[i];
+ secp256k1_pubkey result;
+ pubkeys[i] = NULL;
+ CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_combine(CTX, &result, pubkeys, 3));
+ pubkeys[i] = original_ptr;
+ }
len = 33;
CHECK(secp256k1_ec_pubkey_serialize(CTX, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
CHECK(secp256k1_ec_pubkey_serialize(CTX, ctmp2, &len, &pubkey_one, SECP256K1_EC_COMPRESSED) == 1);
@@ -6640,6 +6649,7 @@ static void permute(size_t *arr, size_t n) {
static void test_sort_api(void) {
secp256k1_pubkey pks[2];
const secp256k1_pubkey *pks_ptr[2];
+ int i;
pks_ptr[0] = &pks[0];
pks_ptr[1] = &pks[1];
@@ -6648,6 +6658,13 @@ static void test_sort_api(void) {
testutil_random_pubkey_test(&pks[1]);
CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2) == 1);
+ /* check that NULL in array of public key pointers is not allowed */
+ for (i = 0; i < 2; i++) {
+ const secp256k1_pubkey *original_ptr = pks_ptr[i];
+ pks_ptr[i] = NULL;
+ CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2));
+ pks_ptr[i] = original_ptr;
+ }
CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_sort(CTX, NULL, 2));
CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 0) == 1);
/* Test illegal public keys */
Why this scored 12/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.