fix: Bound the per-connection peer state cache - #118
Open
giaki3003 wants to merge 5 commits into
Conversation
…ers` response read budget (memory-exhaustion sync DoS) Bug: w3-20260618-1639-kimiclaw-confirm-glmclaw-t (primary) Finding: findings/20260618-1639-kimiclaw-confirm-glmclaw-thunder-rust-getheaders-response-limit-inflation.md Severity: R3-T2 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit a49be1d)
Bug: w3-20260618-1609-kimiclaw-confirm-openclaw- (primary) Finding: findings/20260618-1609-kimiclaw-confirm-openclaw-thunder-rust-peer-state-heartbeat-churn.md Severity: R3-T3 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit bbb6c55)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's wrong
ConnectionTask::handle_peer_request(lib/net/peer/task.rs) inserts every heartbeat'sPeerStateinto a per-connectionHashMap<PeerStateId, PeerState>, and nothing ever removes entries. The peer composes its own heartbeats, so it can produce an unlimited number of distinct states (varyingversionortip_infois enough) on a single connection. The map then grows for the lifetime of that connection, at whatever rate the peer is willing to send heartbeats.The fix
The map becomes a
hashlink::LinkedHashMap(already a dependency), and all inserts go through a newinsert_peer_statehelper that appends at the back and pops from the front once the map exceedsMAX_PEER_STATES(256), so the most recently received states are the ones retained.Because a state can now be evicted, the two
Error::MissingPeerStatereturns inhandle_internal_messagewould otherwise tear down an otherwise healthy connection; they now log a warning and drop the message instead. 256 is a judgement call — happy to change it, or to make it configurable.Tests
Adds
peer_state_cache_is_bounded, which inserts2 * MAX_PEER_STATESdistinct states and checks that the cache stays atMAX_PEER_STATESand holds exactly the most recent ones.Finding report (access-controlled): https://giaki3003.tech/#/findings/20260618-1609-kimiclaw-confirm-openclaw-thunder-rust-peer-state-heartbeat-churn
Part of a short series for this repo (fix 2 of 3); builds on #117, so it reads best merged after that one. Happy to rebase or split if you'd prefer them independent.