in reviewhumanPR #191Transport
relay: Fix HotStuff locked-QC bypass via omitted justify_qc
If you operate BFT relay infrastructure in production, code review isn't where you find safety bugs — you find them by asking "what does a leader that's lying cost me, and does anything notice?" That's the audit I ran against…
Author
@Ajskskdaf
github profile →- Status
- In review
- Opened on
- Jul 11
- Branch
- hackathon/relay-hotstuff-locked-qc-bypass
Description
The pitch.
## Why this, and how I looked for it
If you operate BFT relay infrastructure in production, code review isn't where you find safety bugs — you find them by asking "what does a leader that's lying cost me, and does anything notice?" That's the audit I ran against `nest_core.scenarios_builtin.bft_hotstuff`: walk the safety-critical path (`ReplicaAgent._handle_prepare`), find the one guard standing between "leader proposes" and "replica votes," and ask what a leader gets away with if it just... doesn't send what that guard checks for.
## What I found
`_handle_prepare` only compared `justify_qc.view` against the replica's lock when a `justify_qc` was present *at all*. Omit `justify_qc` entirely and the whole check short-circuits — a leader dodges the locked-QC rule just by leaving a field blank. That rule exists specifically to stop a leader from silently discarding an already prepare-QC'd ("locked") value after a view-change without justifying why — the module's own docstring says as much ("never vote prepare for a view below what you've already locked," the locked-QC rule from Yin et al. 2019, HotStuff, sec 4.3). A guard that's this easy to route around isn't a guard.
## Blast radius, measured, not assumed
I didn't want to ship a fix on a hunch, so I built the attacker (`NoJustifyLeaderAgent`) and ran it against the real code before touching the fix:
- **Pre-fix**: 6 of the malicious leader's bypass attempts (views 8, 15, 22, 29, 36, 43) were fully accepted and committed by honest replicas — a byzantine leader rewrote already-quorum-certified history 6 times in one ~4000-tick run, and every honest replica went along with it silently. No alarms, no dissent, nothing in the trace flags it unless you already know to look.
- **Post-fix**: 0 of 16 bypass attempts got committed. Every one was rejected the moment a lock existed.
That gap — a real leader successfully overwriting agreed history, invisibly, repeatedly — is exactly the failure mode a relay operator loses sleep over: not "does it crash," but "does it silently disagree with itself and nobody notices until reconciliation."
## The fix
Treat a missing `justify_qc` as strictly lower than any real lock, so it's rejected exactly like a too-low `justify_qc` once `locked_qc` is set. Only the unlocked genesis case still passes with no `justify_qc`.
## What's added
- **`NoJustifyLeaderAgent`** — a malicious leader that always proposes with `justify_qc=None` on its own leader turns (`task.config.malicious_kind: no_justify`), exercising the exact bypass path.
- **`validate_bft_locked_qc_respected`** — a new adversarial trace validator that flags any proposal made after a prior lock exists without an adequate `justify_qc`. Like `bft_no_equivocation`, it's a "did the attack happen" detector: it correctly fails on the new scenario (the malicious leader keeps trying every turn) while the safety/liveness validators (`bft_no_conflicting_commits`, `bft_forged_quorum`, `bft_no_stuck_view`) all still pass — proof the fix stops the attack from causing damage, not just proof the attempt gets logged.
- **`scenarios/bft_consensus_locked_qc_bypass.yaml`** — 7 replicas (f=2), 2 leaders running the bypass attack, plus 28% byzantine transport noise, deterministic across seeds 42/7/1337/0xDEADBEEF.
- **Regression tests** in `test_bft_hotstuff_scenario.py`: a scenario-level test proving safety/liveness hold despite repeated bypass attempts, and unit tests pinning the validator's detection logic against hand-built traces.
## Verification
Reverted just the fix locally and reran the new tests against the old code to confirm the test suite actually depends on it — not a no-op regression test dressed up to look rigorous. Full `make ci-local` sequence passes: `ruff check`, `ruff format --check`, `pyright` (0 errors), full `pytest` suite (1288 passed, 1 skipped). Run it yourself with: `uv run pytest -v packages/nest-core/tests/test_bft_hotstuff_scenario.py`
Try it
Open PR on GitHubView diffCheckout locally
git fetch origin pull/191/head:pr-191
git checkout pr-191