security(hello): gate outbound FetchTipSet on strictly-heavier peer weight - #13701
security(hello): gate outbound FetchTipSet on strictly-heavier peer weight#13701MattHintz wants to merge 1 commit into
Conversation
…eight Before this change, the hello handler dispatched an outbound chain-exchange FetchTipSet whenever any peer's HelloMessage decoded successfully, without verifying that the peer's advertised HeaviestTipSetWeight was actually heavier than the node's own local chain weight. An unauthenticated attacker advertising an arbitrary weight could therefore pin one handler goroutine and one outbound-stream slot per hello for the full FetchTipSet deadline, repeated across N attacker-controlled peer IDs. With the gate in place, a peer that legitimately advertises heavier weight must have real storage power backing that weight; a Sybil cannot fabricate weight without producing blocks. See: https://gist.github.com/MattHintz/3f4a1a82ef60cea6ea2df0fc9751426d
There was a problem hiding this comment.
Please update the PR title to match https://github.com/filecoin-project/lotus/blob/master/CONTRIBUTING.md#pr-title-conventions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71ddbf4d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "peer", s.Conn().RemotePeer(), "err", werr) | ||
| return | ||
| } | ||
| if !hmsg.HeaviestTipSetWeight.GreaterThan(ourWeight) { |
There was a problem hiding this comment.
Do not trust the advertised weight for this gate
Because HeaviestTipSetWeight comes directly from the peer's hello message and is not tied to HeaviestTipSet before this check, a malicious peer can advertise any value greater than ourWeight while using a random or unfetchable tipset key. That still passes this branch and reaches FetchTipSet, so the handler and outbound stream can be pinned in the same untrusted-peer scenario this change is intended to block.
Useful? React with 👍 / 👎.
Summary
Gate the outbound
FetchTipSetdispatched from the hello handler on the peer's advertisedHeaviestTipSetWeightbeing strictly greater than the node's local chain weight. Without the gate any peer that can dial the node can pin one handler goroutine and one outbound-stream slot for the fullFetchTipSetdeadline (5 s) per hello, by advertising an arbitrary weight.The gate uses the same
hs.cs.GetHeaviestTipSet()+hs.cs.Weight(ctx, ...)primitives already used bySayHelloin the same file, so this is a two-primitive local change with no new dependencies.Motivation and evidence
Full disclosure article, source-code audit, and 4-phase paired-control measurements are published in the following gist:
Headline measurements on a mainnet-state Lotus
v1.36.0daemon (Forest snapshot at height 6,049,680), 50 attacker peer IDs, no rcmgr rejections across ~5.77M attacker streams:ChainGetBlockMessagesp50 (1.46 ms baseline → 633 ms under attack)Coordinated disclosure ran May 26 – July 6 2026 via Immunefi (reports #79973, #80341). The case closed under the program's Sybil-attacks exclusion clause, not on technical merits. The two code paths involved (
node/hello/hello.goandchain/exchange/server.go) are unchanged atv1.36.0HEAD. This PR addresses the first of three fixes proposed in the disclosure article; the other two (per-peer hello concurrency cap; cost-aware chain-exchange throttle) will follow as separate PRs.What this PR does not do
This PR closes the hello arm of the compound attack. It does not close the chain-exchange arm on its own. A chain-exchange-server work-budget cap is required to fully close the amplification; that will be submitted as a separate change.
Correctness
GreaterThan, matching the existing pattern inchain/sync_manager.go(isHeavier, line 495) andchain/sync.go.!hmsg.HeaviestTipSetWeight.GreaterThan(ourWeight)so equal weights are also skipped, matching the semantics of the existingSayHelloweight-report path.ourHeaduseshs.cs.GetHeaviestTipSet()which is a cheap in-memory read against the chain store head; no additional I/O is introduced on the fast path.Backwards compatibility
No protocol wire changes. No changes to the
HelloMessagestruct or toSayHello. Legitimate peers that advertise heavier weight are unaffected. Peers that advertise equal-or-lighter weight now skip the outbound fetch on the receiving side — this matches what the receiving node would do anyway onceFetchTipSetreturned, becauseInformNewHeadis only called whents.TipSet().Height() > 0and the sync manager only advances on strictly-heavier weight.Alternatives considered
Testing
The disclosure gist includes:
measure_sync_stall.py— measurement harness forChainGetBlockMessageslatency, CPU, RSSrun_refile_phases.sh— 4-phase experiment orchestrator (P0 baseline, P1 legit-only, P2 sybil-only, P3 compound+legit)isolated_config.toml— isolated-daemon configI would recommend maintainers reproduce phase P0 vs P3 against a mainnet-state daemon before and after this patch. The expected result with the patch applied: hello-arm handler goroutines no longer block on
FetchTipSet, and the RSS/CPU/latency amplification collapses substantially. The chain-exchange arm remains until Fix 3 lands.Related
Reported by @MattHintz (Immunefi handle: Venator).