frost/secp256k1-zkp
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chilldkg: harden persisted-state bounds checks for noverify builds

Browse Source 
The four opaque-state loaders (participant state1, state2,
coordinator state, inv_data) guarded the t/n/participant_id fields
read from the serialized object with VERIFY_CHECK, which is compiled
out in noverify (release) builds. chilldkg.md explicitly recommends
persisting state1 and the coordinator state between rounds, which
turns the persisted file into an attack surface: a state1 with n
altered to 100000 and the magic bytes left intact crashed
participant_step2 with out-of-bounds array indexing in a noverify
build.

Promote all four guards to ARG_CHECK, consistent with the magic-byte
check right above them: the illegal-argument callback fires, the load
returns 0, and the public entry points (participant_step2,
participant_finalize, coordinator_finalize, participant_investigate)
return SECP256K1_CHILLDKG_INVALID_INPUT before any state-derived size
is used -- verified in both verify and noverify builds.
This commit is contained in:
Kgothatso Ngako
2026-08-31 13:25:26 +02:00
parent c43f645f15
commit ceccb50a5a
1 changed files with 30 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
src/modules/chilldkg/main_impl.h
View File

@@ -102,8 +102,11 @@ static int secp256k1_chilldkg_participant_inv_data_load(const secp256k1_context
ptr += 4; ptr += 4;
inv_data_i->simpl.participant_id = secp256k1_read_be32(ptr); inv_data_i->simpl.participant_id = secp256k1_read_be32(ptr);
ptr += 4; ptr += 4;
/* The remaining contents were written by inv_data_save. */ /* The remaining contents were written by inv_data_save. The bounds check
VERIFY_CHECK(inv_data_i->simpl.n >= 1 * 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.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS
&& inv_data_i->simpl.participant_id < inv_data_i->simpl.n); && inv_data_i->simpl.participant_id < inv_data_i->simpl.n);
secp256k1_scalar_set_b32(&inv_data_i->simpl.secshare, ptr, NULL); secp256k1_scalar_set_b32(&inv_data_i->simpl.secshare, ptr, NULL);
@@ -163,8 +166,11 @@ static int secp256k1_chilldkg_participant_state1_load(const secp256k1_context *c
ptr += 4; ptr += 4;
state_i->simpl_state.participant_id = secp256k1_read_be32(ptr); state_i->simpl_state.participant_id = secp256k1_read_be32(ptr);
ptr += 4; ptr += 4;
/* The remaining contents were written by state1_save. */ /* The remaining contents were written by state1_save. The bounds check
VERIFY_CHECK(state_i->simpl_state.t >= 1 * 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.t <= state_i->simpl_state.n
&& state_i->simpl_state.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS && state_i->simpl_state.n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS
&& state_i->simpl_state.participant_id < state_i->simpl_state.n); && state_i->simpl_state.participant_id < state_i->simpl_state.n);
@@ -226,8 +232,11 @@ static int secp256k1_chilldkg_participant_state2_load(const secp256k1_context *c
ptr += 4; ptr += 4;
state_i->n = secp256k1_read_be32(ptr); state_i->n = secp256k1_read_be32(ptr);
ptr += 4; ptr += 4;
/* The remaining contents were written by state2_save. */ /* The remaining contents were written by state2_save. The bounds check
VERIFY_CHECK(state_i->t >= 1 * 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->t <= state_i->n
&& state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS); && state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS);
memcpy(state_i->eq_input, ptr, sizeof(state_i->eq_input)); memcpy(state_i->eq_input, ptr, sizeof(state_i->eq_input));
@@ -569,8 +578,12 @@ static int secp256k1_chilldkg_coordinator_state_load(const secp256k1_context *ct
ptr += 4; ptr += 4;
state_i->n = secp256k1_read_be32(ptr); state_i->n = secp256k1_read_be32(ptr);
ptr += 4; ptr += 4;
/* The remaining contents were written by coordinator_state_save. */ /* The remaining contents were written by coordinator_state_save. The
VERIFY_CHECK(state_i->t >= 1 * 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->t <= state_i->n
&& state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS); && state_i->n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS);
memcpy(state_i->eq_input, ptr, sizeof(state_i->eq_input)); memcpy(state_i->eq_input, ptr, sizeof(state_i->eq_input));