chilldkg: add regression tests for the recovery/state bounds fixes
Add chilldkg_input_bounds_test covering the two review findings that the vector suite cannot catch (the memory-safe Python reference never exercises fixed-size-array overflows, so no upstream vector encodes them): - Recovery data with t = 129 (one past MAX_PARTICIPANTS) and t = 30000, both with n = 0: previously a stack buffer overflow in deserialize_recovery_data (ASan-confirmed: this test reports a stack-buffer-overflow on sum_coms against the unfixed code); now both participant_recover and coordinator_recover return INVALID_INPUT with outputs zeroed. Plus a t = 0 case. - Tampered persisted state objects (valid magic, n = 100000) for all four state types: participant_step2, participant_finalize, coordinator_finalize and participant_investigate fire the illegal-argument callback and return INVALID_INPUT instead of indexing out of bounds, in both verify and noverify builds. Verified: the test fails (ASan stack-buffer-overflow) against the pre-fix parser and passes against the fixed code, in the default, noverify and ASan builds.
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user