diff --git a/src/modules/chilldkg/tests_impl.h b/src/modules/chilldkg/tests_impl.h index 1d536a20..089013d2 100644 --- a/src/modules/chilldkg/tests_impl.h +++ b/src/modules/chilldkg/tests_impl.h @@ -2268,6 +2268,109 @@ static void chilldkg_frost_integration_test(void) { } #endif +/* Regression tests for two review findings the vector suite cannot catch + * (the Python reference is memory-safe and has no fixed-size arrays, so no + * vector exercises them): + * 1. The threshold t in untrusted recovery data was used directly as a loop + * bound over the fixed-size sum_coms[SECP256K1_CHILLDKG_MAX_PARTICIPANTS] + * array in deserialize_recovery_data: a blob with t = 129 wrote one group + * element past the array, t = 30000 wrote ~1.2 MB past it. Reachable from + * both participant_recover and coordinator_recover with attacker-supplied + * recovery data. + * 2. The bounds on t/n in the persisted-state loaders were VERIFY_CHECKs, + * which are compiled out in noverify builds: a state1 with n altered to + * 100000 (magic left intact) crashed participant_step2. */ +static void chilldkg_input_bounds_test(void) { + static unsigned char recovery_big[4 + 33 * 30000]; /* 990004 bytes */ + unsigned char secshare32[32]; + unsigned char thresh_pk33[33]; + unsigned char pubshares33[33 * SECP256K1_CHILLDKG_MAX_PARTICIPANTS]; + unsigned char hostpubkeys33[33 * SECP256K1_CHILLDKG_MAX_PARTICIPANTS]; + unsigned char sig64[64] = { 0 }; + unsigned char aux_rand32[32] = { 0 }; + unsigned char cmsg1_dummy[357] = { 0 }; /* cmsg1 size for n = 2, t = 2 */ + unsigned char cmsg2_dummy[64] = { 0 }; + unsigned char cinv_dummy[130] = { 0 }; /* 65 * 2 */ + unsigned char recovery_dummy[556] = { 0 }; + const unsigned char *pmsgs2_dummy[1]; + secp256k1_chilldkg_fault fault; + size_t n_out; + uint32_t t_out, fault_index; + + pmsgs2_dummy[0] = sig64; + + /* Out-of-range thresholds in recovery data are rejected as invalid input + * (RecoveryDataError in the reference), with the outputs zeroed. t = 129 + * is one past SECP256K1_CHILLDKG_MAX_PARTICIPANTS; t = 30000 is the + * large-overflow case. In both, n computes to 0. */ + memset(recovery_big, 0, sizeof(recovery_big)); + secp256k1_write_be32(recovery_big, 129); + memset(secshare32, 1, 32); + memset(thresh_pk33, 1, 33); + n_out = 77; + t_out = 77; + fault = secp256k1_chilldkg_participant_recover(CTX, secshare32, thresh_pk33, pubshares33, hostpubkeys33, &n_out, &t_out, &fault_index, vec5_hostseckeys[0], recovery_big, 4 + 33 * 129); + CHECK(fault == SECP256K1_CHILLDKG_INVALID_INPUT); + CHECK(secp256k1_is_zero_array(secshare32, 32)); + CHECK(secp256k1_is_zero_array(thresh_pk33, 33)); + CHECK(n_out == 0 && t_out == 0); + fault = secp256k1_chilldkg_coordinator_recover(CTX, thresh_pk33, pubshares33, hostpubkeys33, &n_out, &t_out, recovery_big, 4 + 33 * 129); + CHECK(fault == SECP256K1_CHILLDKG_INVALID_INPUT); + + secp256k1_write_be32(recovery_big, 30000); + fault = secp256k1_chilldkg_participant_recover(CTX, secshare32, thresh_pk33, pubshares33, hostpubkeys33, &n_out, &t_out, &fault_index, vec5_hostseckeys[0], recovery_big, 4 + 33 * 30000); + CHECK(fault == SECP256K1_CHILLDKG_INVALID_INPUT); + fault = secp256k1_chilldkg_coordinator_recover(CTX, thresh_pk33, pubshares33, hostpubkeys33, &n_out, &t_out, recovery_big, 4 + 33 * 30000); + CHECK(fault == SECP256K1_CHILLDKG_INVALID_INPUT); + + /* t = 0 is invalid as well (the session parameters require 1 <= t). */ + secp256k1_write_be32(recovery_big, 0); + fault = secp256k1_chilldkg_participant_recover(CTX, secshare32, thresh_pk33, pubshares33, hostpubkeys33, &n_out, &t_out, &fault_index, vec5_hostseckeys[0], recovery_big, 4 + 162 * 3); + CHECK(fault == SECP256K1_CHILLDKG_INVALID_INPUT); + + /* Tampered persisted state: valid magic, but n = 100000. The loaders + * reject it in all build modes (the illegal-argument callback fires), the + * public functions return INVALID_INPUT, and nothing is indexed out of + * bounds. */ + { + secp256k1_chilldkg_participant_state1 state1; + secp256k1_chilldkg_participant_state2 state2; + secp256k1_chilldkg_coordinator_state cstate; + secp256k1_chilldkg_participant_inv_data inv_data; + + memset(&state1, 0, sizeof(state1)); + memcpy(state1.data, secp256k1_chilldkg_participant_state1_magic, 4); + secp256k1_write_be32(state1.data + 4, 1); /* t */ + secp256k1_write_be32(state1.data + 8, 100000); /* n */ + secp256k1_write_be32(state1.data + 12, 0); /* participant_id */ + fault_index = 0; + CHECK_ILLEGAL_VOID(CTX, CHECK(secp256k1_chilldkg_participant_step2(CTX, &state2, sig64, &fault_index, NULL, &state1, vec5_hostseckeys[0], cmsg1_dummy, aux_rand32) == SECP256K1_CHILLDKG_INVALID_INPUT)); + CHECK(fault_index == UINT32_MAX); + + memset(&state2, 0, sizeof(state2)); + memcpy(state2.data, secp256k1_chilldkg_participant_state2_magic, 4); + secp256k1_write_be32(state2.data + 4, 1); /* t */ + secp256k1_write_be32(state2.data + 8, 100000); /* n */ + fault_index = 0; + CHECK_ILLEGAL_VOID(CTX, CHECK(secp256k1_chilldkg_participant_finalize(CTX, secshare32, thresh_pk33, pubshares33, recovery_dummy, &fault_index, &state2, cmsg2_dummy) == SECP256K1_CHILLDKG_INVALID_INPUT)); + + memset(&cstate, 0, sizeof(cstate)); + memcpy(cstate.data, secp256k1_chilldkg_coordinator_state_magic, 4); + secp256k1_write_be32(cstate.data + 4, 1); /* t */ + secp256k1_write_be32(cstate.data + 8, 100000); /* n */ + fault_index = 0; + CHECK_ILLEGAL_VOID(CTX, CHECK(secp256k1_chilldkg_coordinator_finalize(CTX, cmsg2_dummy, thresh_pk33, pubshares33, recovery_dummy, &fault_index, &cstate, pmsgs2_dummy) == SECP256K1_CHILLDKG_INVALID_INPUT)); + + memset(&inv_data, 0, sizeof(inv_data)); + memcpy(inv_data.data, secp256k1_chilldkg_participant_inv_data_magic, 4); + secp256k1_write_be32(inv_data.data + 4, 100000); /* n */ + secp256k1_write_be32(inv_data.data + 8, 0); /* participant_id */ + fault_index = 0; + CHECK_ILLEGAL_VOID(CTX, CHECK(secp256k1_chilldkg_participant_investigate(CTX, &fault_index, &inv_data, cinv_dummy) == SECP256K1_CHILLDKG_INVALID_INPUT)); + CHECK(fault_index == UINT32_MAX); + } +} + static const struct tf_test_entry tests_chilldkg[] = { CASE1(chilldkg_tagged_hashes_test), CASE1(chilldkg_params_hash_test), @@ -2284,6 +2387,7 @@ static const struct tf_test_entry tests_chilldkg[] = { CASE1(chilldkg_investigate_test), CASE1(chilldkg_vectors_test), CASE1(chilldkg_boundary_test), + CASE1(chilldkg_input_bounds_test), #ifdef ENABLE_MODULE_FROST CASE1(chilldkg_frost_integration_test), #endif