DKG permanently stuck for validator sets outside {4,5,6}: wrong coefficient-count check in register_round1_package

Bug Bounty Submitted
1

Component: ghost-node @ 0162da4604d9931c16d12a0cec40dd85951368c5 — pallets/exodus/src/lib.rs:982-996 (register_round1_package), pallets/exodus/src/lib.rs:3745-3786 (dkg_run_round1 offchain worker), pallets/helpers/src/lib.rs:37-50 (get_byzantium_quorum_threshold). The production runtime allows up to 300 validators (runtime/casper/src/lib.rs:348, MaxActiveValidators = 300).

Description

The offchain worker generates DKG round-1 packages with min_signers = get_byzantium_quorum_threshold(max_signers) (lib.rs:3750), where the threshold is n - floor((n-1)/3). The VSS commitment in the package therefore carries exactly min_signers coefficients (frost-core 2.2.0 keys::dkg::part1).

The on-chain check in register_round1_package requires something different:

// pallets/exodus/src/lib.rs:991-996
state.count_ones::<AuthIndex>().checked_sub(1)
    .map(|needed_commitment_count| {
        estimated_commitment_count
            .eq(&needed_commitment_count)
            .then(|| ())
    })
    .ok_or(Error::<T>::Round1PackageInvalidLength)?;

i.e. it demands coefficient_count == (round0 registrants) - 1. And dkg_verify_proof_of_knowledge (pallets/exodus/src/impls/dkg.rs:109-111) returns commitment.coefficients().len(), which is min_signers by construction. The two formulas agree only for n in {4,5,6}: n=4: 3==3; n=5: 4==4; n=6: 5==5; n=7: 5!=6; n=8: 6!=7; n=10: 7!=9.

Ironic detail: validate_round1_package_size, used by validate_unsigned, uses the correct formula get_byzantium_quorum_threshold(authorities.len()) (lib.rs:2729). So the unsigned transactions pass the transaction pool and then fail at dispatch with Round1PackageInvalidLength, wasting block space on every DKG cycle.

Impact

For any validator set size outside {4,5,6}, DKG can never complete: every register_round1_package fails, prepare_round2_storage intersects the qualification bitmap down to zero, on_initialize (lib.rs:1940-1946) misses quorum, restart_dkg() fires, and the cycle fails forever. No key generation, no rotation, no threshold signing (bridge-out, rotation, governance). This is a total, deterministic liveness kill of the pallet’s core function and it needs no attacker at all. Every test in the repo uses 4 validators (mock.rs: FixedAuthorities = vec![0,1,2,3]), which is exactly the size where both formulas agree, so this slipped through.

Reproduction

Local PoC (no network, no mainnet touched). It builds honest round-1 packages with the same library the pallet uses (frost_secp256k1_tr::keys::dkg::part1), reads commitment.coefficients().len(), and applies the exact on-chain condition from lib.rs:991-996:

n= 4: min_signers(threshold)=3, coeffs=3, on-chain requires count_ones-1=3 -> ACCEPTED
n= 5: min_signers(threshold)=4, coeffs=4, on-chain requires count_ones-1=4 -> ACCEPTED
n= 6: min_signers(threshold)=5, coeffs=5, on-chain requires count_ones-1=5 -> ACCEPTED
n= 7: min_signers(threshold)=5, coeffs=5, on-chain requires count_ones-1=6 -> REJECTED (Round1PackageInvalidLength)
n= 8: min_signers(threshold)=6, coeffs=6, on-chain requires count_ones-1=7 -> REJECTED (Round1PackageInvalidLength)
n=10: min_signers(threshold)=7, coeffs=7, on-chain requires count_ones-1=9 -> REJECTED (Round1PackageInvalidLength)

A full integration test poc_round1_package_rejected_with_seven_authorities was also added to pallets/exodus/src/tests.rs and passes.

Severity: High

Reason: deterministic total liveness kill of the threshold-signing pipeline for any validator set outside {4,5,6}, with no attacker required. Not Critical because there is no fund theft or loss path, only denial of the DKG/signing service.

Recommendation

Replace the check with the same threshold the offchain worker uses, e.g. estimated_commitment_count == get_byzantium_quorum_threshold(state.count_ones()) (or against total authorities). Add DKG tests with 7 and 10 validators so the two formulas are exercised where they disagree.

Verification status

VERIFIED. Mechanism confirmed line-by-line in source; PoC executed locally and passed.

2

Thank you for your report!

This one duplicates another report filed here. I know you were the first to report it, but either way, please read the answer there to avoid a copy-paste from my side.

To summarize: this is not a bug as described, because the check effectively verifies only checked_sub. The coefficient-count comparison is never enforced - the expression .map(...).ok_or(...)? operates on Option<Option<()>>, so the inner Option produced by .then(|| ()) is silently discarded. The gate only rejects when count_ones == 0.

If you think this issue could be elevated in other ways, please continue the thread.

3

You’re right, and I’m withdrawing this report.

Re-reading register_round1_package (lib.rs:991), the check is:

state.count_ones::<AuthIndex>().checked_sub(1)
    .map(|needed| estimated_commitment_count.eq(&needed).then(|| ()))
    .ok_or(Error::<T>::Round1PackageInvalidLength)?;

The .map over the Option from checked_sub produces Option<Option<()>>; ok_or turns it into Result<Option<()>, _>, and ? silently discards the inner comparison. The coefficient-count equality is never enforced. The only rejection path is count_ones == 0, which cannot happen while a DKG session is running. Honest packages are accepted at any validator set size, so the deterministic brick I described does not exist.

My PoC replicated the check as I misread it (an enforced comparison) rather than as written, so the passing PoC proved my misreading, not the chain’s behavior. That’s on me.

As a code-hygiene note, not a finding: if the intent is to enforce the count, this wants a plain ensure! on the boolean; as written the gate is dead code. Withdrawing.

4

Looks like it partially fixed here. At the same time @Chaplin slightly deviate from the issue described here. Can you guys confirm on is it the same issue or they are different?

5

They’re related but distinct — two failure modes of the same check in register_round1_package.

#99 is about false negatives: the pre-fix check (count_ones - 1) disagrees with what the offchain worker actually puts in the package (get_byzantium_quorum_threshold), so every honest round-1 package is rejected with Round1PackageInvalidLength for n outside {4,5,6}. I reproduced this independently — the formulas only agree at n=4,5,6, and the worker’s coefficient count is threshold(n) by construction (frost dkg::part1 with min_signers). DKG can never complete; that’s the liveness kill described in #99.

H16 (#123) was about the other direction — false positives. The same length check could be bypassed with trailing-byte smuggling, letting a malformed package through the gate. Different attacker model, different consequence.

Commit 653bf0848 replaces the check wholesale with estimated_commitment_count == get_byzantium_quorum_threshold(max_signers), which is exactly the formula #99 recommended. That one change closes both: honest packages are accepted at every n (fixing #99), and the smuggling bypass no longer works (fixing H16’s round-1 vector).

So: same line of code, different bugs, one fix covers both.