Skip to content

fix: bind checkpoint transition queues to the verified terminal header#423

Merged
matthias-wright merged 1 commit into
audit-may-2026from
m/epoch-transition-queues-ckpt
Jun 30, 2026
Merged

fix: bind checkpoint transition queues to the verified terminal header#423
matthias-wright merged 1 commit into
audit-may-2026from
m/epoch-transition-queues-ckpt

Conversation

@matthias-wright

Copy link
Copy Markdown
Collaborator

Builds on #422 (which builds on #402)

Addresses #216.

Changes:

  • types/src/checkpoint.rs: add Step 5 to verify_checkpoint_chain_with_weak_subjectivity binding the decoded queues to the terminal header's committed deltas — require checkpoint_state.get_added_validators(next_epoch) to equal terminal.added_validators() and checkpoint_state.get_removed_validators() to equal terminal.removed_validators() (order-normalized), rejecting mismatches with a new CheckpointVerificationError::CheckpointTransitionQueueMismatch.
  • Document the inherent residual: pending additions for epochs beyond next_epoch (possible when validator warm-up spans more than one epoch) are committed by finalized headers after the terminal and cannot be authenticated by a chain ending at this epoch; they are bound only once the next epoch's boundary header is verified.
  • Test: add test_checkpoint_verifier_binds_transition_queues_to_terminal_header (honest empty-queue checkpoint verifies; a checkpoint carrying a pending removed_validators entry while the terminal header commits an empty set is rejected, with hash/membership/position checks still passing). Parameterize checkpoint_verification_fixture with the checkpoint's removed_validators queue and update its callers.

@sebastian-osec

Copy link
Copy Markdown

I think the immediate transition check is correct, but I’m not sure we should accept future added_validators buckets as trusted checkpoint contents.

For a checkpoint at epoch N, the terminal finalized header authenticates removed_validators and added_validators[N + 1]. But ConsensusState.added_validators is a map, and entries for N + 2 or later are serialized inside the checkpoint and are not checked here.

Given VALIDATOR_NUM_WARM_UP_EPOCHS = 2, this seems reachable for normal deposits: a checkpoint at epoch N can carry added_validators[N + 2]. A malicious checkpoint could keep added_validators[N + 1] correct while choosing future additions, and Step 5 would still pass.

Could we either reject any added_validators entries beyond next_epoch, or explicitly document that future additions remain trusted checkpoint contents and are outside the fix for #216?

@matthias-wright matthias-wright force-pushed the m/epoch-transition-queues-ckpt branch from 99267a0 to 872aac1 Compare June 30, 2026 13:53
@matthias-wright

Copy link
Copy Markdown
Collaborator Author

Good point, I added a comment in types/src/checkpoint.rs to clarify this.

@matthias-wright matthias-wright merged commit 4b2a072 into audit-may-2026 Jun 30, 2026
4 checks passed
@matthias-wright matthias-wright deleted the m/epoch-transition-queues-ckpt branch June 30, 2026 14:20
matthias-wright added a commit that referenced this pull request Jul 2, 2026
…425)

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.
-Bind checkpoint consensus keys to the verified terminal header.
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