Skip to content

fix: bind checkpoint consensus keys to the verified terminal header#425

Merged
matthias-wright merged 2 commits into
audit-may-2026from
m/ckpt-bls-verify
Jul 2, 2026
Merged

fix: bind checkpoint consensus keys to the verified terminal header#425
matthias-wright merged 2 commits into
audit-may-2026from
m/ckpt-bls-verify

Conversation

@matthias-wright

Copy link
Copy Markdown
Collaborator

Builds on #423 (which builds on #422 and #402).

Addresses #217.

Changes:

  • Step 3 now accumulates a node-key to BLS-key map and requires each checkpoint account's consensus_public_key to equal the BLS key accumulated for that node from the verified history. Mismatch returns the new CheckpointConsensusKeyMismatch error variant. The reverse membership check uses the same map.
  • Add test_checkpoint_verifier_binds_consensus_keys_to_terminal_header covering both directions: the honest case (accounts carry the real, accumulated consensus keys and verify) and a swapped consensus key (rejected with CheckpointConsensusKeyMismatch). The shared checkpoint_verification_fixture gains a tamper_consensus_key parameter that swaps one active account's stored consensus key while leaving its node key, status, and the accumulated set intact.

@sebastian-osec

Copy link
Copy Markdown

I think this is a good partial fix for the current active-validator key-swap case, but I think there is still a bypass for validators that are queued to join.

The new check binds validator_accounts[node_key].consensus_public_key to the accumulated BLS key only for validators in the current signing set. A Joining validator is not in that signing set yet, so its account key is not checked against the verified added_validators[next_epoch] entry.

That matters because epoch activation later loads the account by node key and only changes:

account.status = ValidatorStatus::Active;

It does not overwrite account.consensus_public_key from the verified AddedValidator.consensus_key.

So a malicious checkpoint can make added_validators[next_epoch] match the terminal header, while setting the corresponding joining account’s consensus_public_key to a different BLS key. Verification accepts it, and when the validator activates, the attacker-chosen account key becomes active.

I think the verifier should also enforce:

validator_accounts[added.node_key].consensus_public_key == added.consensus_key

for every validator in added_validators[next_epoch].

@matthias-wright

Copy link
Copy Markdown
Collaborator Author

Update(bfc4fc9):

  • Bind checkpoint consensus keys to the verified terminal header

@sebastian-osec

Copy link
Copy Markdown

This looks good to me now.

@matthias-wright matthias-wright merged commit dfb7138 into audit-may-2026 Jul 2, 2026
4 checks passed
@matthias-wright matthias-wright deleted the m/ckpt-bls-verify branch July 2, 2026 05:31
matthias-wright added a commit that referenced this pull request Jul 2, 2026
Builds on #425 (which builds on #423, #422, and #402).

Addresses #310.

This issue was already solved by #170 and #286.

This PR just adds a regression test.

Changes:
-Add regression for finalized-header epoch-replay rejection
matthias-wright added a commit that referenced this pull request Jul 2, 2026
Builds on #426 (which builds on #425, #423, #422, and #402).

Addresses #311.

This only adds test coverage, the issue is not reachable.
The decoded ConsensusState, including every validator_accounts entry, is committed by checkpoint.data, which verify_checkpoint_chain binds to the terminal finalized header via checkpoint_hash == sha256(checkpoint.data). Appending a Joining account changes the digest, so the tampered checkpoint no longer matches the honest terminal header and is rejected at Step 2.

Changes:
-types/src/checkpoint.rs: test_checkpoint_verifier_rejects_extra_joining_account — reproduces the attack directly. It decodes the honest checkpoint, injects an extra Joining account (leaving the active signing set untouched), re-encodes, and asserts verify_checkpoint_chain rejects it with CheckpointHashMismatch.
-application/src/actor.rs: rejects_block_with_mismatched_checkpoint_hash — the consensus-layer half. It mirrors accepts_ordinary_child_inside_epoch, changing only the block's checkpoint_hash, and asserts handle_verify returns false, isolating the checkpoint_hash check as the sole cause.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants