diff --git a/src/modules/chilldkg/main_impl.h b/src/modules/chilldkg/main_impl.h index 122911a4..d1002c5a 100644 --- a/src/modules/chilldkg/main_impl.h +++ b/src/modules/chilldkg/main_impl.h @@ -102,10 +102,13 @@ static int secp256k1_chilldkg_participant_inv_data_load(const secp256k1_context ptr += 4; inv_data_i->simpl.participant_id = secp256k1_read_be32(ptr); ptr += 4; - /* The remaining contents were written by inv_data_save. */ - VERIFY_CHECK(inv_data_i->simpl.n >= 1 - && inv_data_i->simpl.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS - && inv_data_i->simpl.participant_id < inv_data_i->simpl.n); + /* The remaining contents were written by inv_data_save. The bounds check + * must not be a VERIFY_CHECK: a persisted inv_data object can be tampered + * with, and in noverify builds the check would be compiled out and the + * out-of-range n would index fixed-size arrays below and in callers. */ + ARG_CHECK(inv_data_i->simpl.n >= 1 + && inv_data_i->simpl.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS + && inv_data_i->simpl.participant_id < inv_data_i->simpl.n); secp256k1_scalar_set_b32(&inv_data_i->simpl.secshare, ptr, NULL); ptr += 32; /* This load always succeeds; call it unconditionally (it must not sit @@ -163,11 +166,14 @@ static int secp256k1_chilldkg_participant_state1_load(const secp256k1_context *c ptr += 4; state_i->simpl_state.participant_id = secp256k1_read_be32(ptr); ptr += 4; - /* The remaining contents were written by state1_save. */ - VERIFY_CHECK(state_i->simpl_state.t >= 1 - && state_i->simpl_state.t <= state_i->simpl_state.n - && state_i->simpl_state.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS - && state_i->simpl_state.participant_id < state_i->simpl_state.n); + /* The remaining contents were written by state1_save. The bounds check + * must not be a VERIFY_CHECK: a persisted state object can be tampered + * with, and in noverify builds the check would be compiled out and the + * out-of-range n would index fixed-size arrays in callers. */ + ARG_CHECK(state_i->simpl_state.t >= 1 + && state_i->simpl_state.t <= state_i->simpl_state.n + && state_i->simpl_state.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS + && state_i->simpl_state.participant_id < state_i->simpl_state.n); /* This load always succeeds; call it unconditionally (it must not sit * inside VERIFY_CHECK, which is compiled out in noverify builds). */ if (!secp256k1_chilldkg_point_load(&state_i->simpl_state.com_to_secret, ptr)) { @@ -226,10 +232,13 @@ static int secp256k1_chilldkg_participant_state2_load(const secp256k1_context *c ptr += 4; state_i->n = secp256k1_read_be32(ptr); ptr += 4; - /* The remaining contents were written by state2_save. */ - VERIFY_CHECK(state_i->t >= 1 - && state_i->t <= state_i->n - && state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS); + /* The remaining contents were written by state2_save. The bounds check + * must not be a VERIFY_CHECK: a persisted state object can be tampered + * with, and in noverify builds the check would be compiled out and the + * out-of-range t/n would index fixed-size arrays in callers. */ + ARG_CHECK(state_i->t >= 1 + && state_i->t <= state_i->n + && state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS); memcpy(state_i->eq_input, ptr, sizeof(state_i->eq_input)); ptr += sizeof(state_i->eq_input); memcpy(state_i->dkg_output.secshare32, ptr, 32); @@ -569,10 +578,14 @@ static int secp256k1_chilldkg_coordinator_state_load(const secp256k1_context *ct ptr += 4; state_i->n = secp256k1_read_be32(ptr); ptr += 4; - /* The remaining contents were written by coordinator_state_save. */ - VERIFY_CHECK(state_i->t >= 1 - && state_i->t <= state_i->n - && state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS); + /* The remaining contents were written by coordinator_state_save. The + * bounds check must not be a VERIFY_CHECK: a persisted state object can + * be tampered with, and in noverify builds the check would be compiled + * out and the out-of-range t/n would index fixed-size arrays in + * callers. */ + ARG_CHECK(state_i->t >= 1 + && state_i->t <= state_i->n + && state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS); memcpy(state_i->eq_input, ptr, sizeof(state_i->eq_input)); ptr += sizeof(state_i->eq_input); memcpy(state_i->thresh_pk33, ptr, 33);