From 5a08c1bcdc958f264450e771d7923922526f74f1 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 6 Dec 2025 01:04:18 +0100 Subject: [PATCH 1/2] Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL --- src/modules/musig/keyagg_impl.h | 3 +++ src/modules/musig/session_impl.h | 8 ++++++++ src/secp256k1.c | 5 +++++ 3 files changed, 16 insertions(+) diff --git a/src/modules/musig/keyagg_impl.h b/src/modules/musig/keyagg_impl.h index 87869a4105..e412a27eca 100644 --- a/src/modules/musig/keyagg_impl.h +++ b/src/modules/musig/keyagg_impl.h @@ -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; diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index 6a80a27b38..4a245ada5f 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -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; @@ -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; diff --git a/src/secp256k1.c b/src/secp256k1.c index c2d3fcbddb..bce06d37fb 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -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) From 8bcda186d20e11ba9a04e6917928f062513b534f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 9 Dec 2025 01:30:56 +0100 Subject: [PATCH 2/2] test: Add non-NULL checks for "pointer of array" API functions --- src/modules/musig/tests_impl.h | 21 +++++++++++++++++++++ src/tests.c | 17 +++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h index b4ba185494..de09c5e280 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 4029de5651..14aa785895 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 */