Verify LaneProposal signer and parent hash chain in PushBlock#2925
Verify LaneProposal signer and parent hash chain in PushBlock#2925wen-coding wants to merge 4 commits intomainfrom
Conversation
PushBlock accepted blocks without checking that the signer matches the lane or that the block's parentHash chains to the previous block. A malicious proposer could impersonate another lane or break the parent hash chain, which would deadlock header reconstruction in headers(). Added signer == lane check (before expensive crypto verification) and parent hash chain verification when the previous block is available. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #2925 +/- ##
===========================================
- Coverage 57.32% 48.37% -8.96%
===========================================
Files 2095 671 -1424
Lines 172888 50611 -122277
===========================================
- Hits 99115 24481 -74634
+ Misses 64880 23982 -40898
+ Partials 8893 2148 -6745
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
A parentHash mismatch can happen legitimately when a producer equivocates (e.g. restarts and produces a different chain). The block is still prevented from entering the queue, but the caller is not treated as malicious. Co-authored-by: Cursor <cursoragent@cursor.com>
| // Verify parent hash chain to prevent a malicious proposer from | ||
| // breaking the block chain, which would deadlock header reconstruction. | ||
| // A mismatch is not an error: it can happen legitimately when a | ||
| // producer equivocates (restarts and produces a different chain). |
There was a problem hiding this comment.
equivocation of producer is as malicious behavior as equivocation of proposer. It does not endanger the security, but imo it would be worth escalating as an error, so that it gets logged. It will help debugging stalled lanes, if nothing more.
There was a problem hiding this comment.
Hmm, if we return an error, and the producer keep pushing the same equivocating block, then every receiver will repeatedly reconnect?
I've changed it to log an error but return nil for now
Parent hash mismatch indicates a producer equivocated, which is malicious behavior worth logging to aid debugging stalled lanes. Co-authored-by: Cursor <cursoragent@cursor.com>
4 participants
Summary
PushBlockdid not verify that the signer of aLaneProposalmatches the block's lane. A malicious validator could craft blocks on another validator's lane, bypassingVerifySig(which only checks the signer is a committee replica, not the lane producer). Added a cheap signer-lane comparison before expensive crypto verification.PushBlockdid not verify that the incoming block'sparentHashmatches the previous block's header hash. A malicious lane proposer could send a block with a fakeparentHash, which honest validators would accept and vote on. Whenheaders()later tries to reconstruct the block chain by walkingparentHashlinks backwards, it cannot find a matching vote and waits forever — deadlocking the node. Added parent hash verification when the previous block is available (after pruning, the check is safely skipped sinceheaders()never follows the first block'sparentHashin aLaneRange). A mismatch silently drops the block rather than returning an error, since equivocation (same key producing different chains) is a legitimate scenario.