Skip to content

Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL #1779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
real-or-random merged 2 commits into from
Dec 10, 2025
+54 −0
Merged

Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL #1779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a08c1b
Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL
theStack Dec 6, 2025
8bcda18
test: Add non-NULL checks for "pointer of array" API functions
theStack Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/modules/musig/keyagg_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ int secp256k1_musig_pubkey_agg(const secp256k1_context* ctx, secp256k1_xonly_pub
}
ARG_CHECK(pubkeys != NULL);
ARG_CHECK(n_pubkeys > 0);
for (i = 0; i < n_pubkeys; i++) {
ARG_CHECK(pubkeys[i] != NULL);
}

ecmult_data.ctx = ctx;
ecmult_data.pks = pubkeys;
Expand Down
8 changes: 8 additions & 0 deletions src/modules/musig/session_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,10 +521,15 @@ static int secp256k1_musig_sum_pubnonces(const secp256k1_context* ctx, secp256k1
int secp256k1_musig_nonce_agg(const secp256k1_context* ctx, secp256k1_musig_aggnonce *aggnonce, const secp256k1_musig_pubnonce * const* pubnonces, size_t n_pubnonces) {
secp256k1_gej aggnonce_ptsj[2];
secp256k1_ge aggnonce_pts[2];
size_t i;

VERIFY_CHECK(ctx != NULL);
ARG_CHECK(aggnonce != NULL);
ARG_CHECK(pubnonces != NULL);
ARG_CHECK(n_pubnonces > 0);
for (i = 0; i < n_pubnonces; i++) {
ARG_CHECK(pubnonces[i] != NULL);
}

if (!secp256k1_musig_sum_pubnonces(ctx, aggnonce_ptsj, pubnonces, n_pubnonces)) {
return 0;
Expand Down Expand Up @@ -782,6 +787,9 @@ int secp256k1_musig_partial_sig_agg(const secp256k1_context* ctx, unsigned char
ARG_CHECK(session != NULL);
ARG_CHECK(partial_sigs != NULL);
ARG_CHECK(n_sigs > 0);
for (i = 0; i < n_sigs; i++) {
ARG_CHECK(partial_sigs[i] != NULL);
}

if (!secp256k1_musig_session_load(ctx, &session_i, session)) {
return 0;
Expand Down
21 changes: 21 additions & 0 deletions src/modules/musig/tests_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
5 changes: 5 additions & 0 deletions src/secp256k1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,13 @@ static int secp256k1_ec_pubkey_sort_cmp(const void* pk1, const void* pk2, void *
}

int secp256k1_ec_pubkey_sort(const secp256k1_context* ctx, const secp256k1_pubkey **pubkeys, size_t n_pubkeys) {
size_t i;

VERIFY_CHECK(ctx != NULL);
ARG_CHECK(pubkeys != NULL);
for (i = 0; i < n_pubkeys; i++) {
ARG_CHECK(pubkeys[i] != NULL);
}

/* Suppress wrong warning (fixed in MSVC 19.33) */
#if defined(_MSC_VER) && (_MSC_VER < 1933)
Expand Down
17 changes: 17 additions & 0 deletions src/tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand All @@ -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 */
Expand Down