Skip to content

fix: set finite gossipsub decode limits for control messages #9117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lodekeeper wants to merge 3 commits into
base: unstable
Choose a base branch
from
+18 −0
Open

fix: set finite gossipsub decode limits for control messages #9117

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39958d2
fix: set finite gossipsub decode limits for control messages
lodekeeper Mar 26, 2026
7a02bc2
fix: raise IHAVE/IWANT decode limits to match GossipsubMaxIHaveLength
lodekeeper Mar 26, 2026
5d43ede
fix: tighten gossipsub decode limit bounds
lodekeeper Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/beacon-node/src/network/gossip/gossipsub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,24 @@ export class Eth2Gossipsub {
// This should be large enough to not send IDONTWANT for "small" messages
// See https://github.com/ChainSafe/lodestar/pull/7077#issuecomment-2383679472
idontwantMinDataSize: 16829,
// Protobuf decode limits to bound memory allocation from untrusted RPC messages.
// js-gossipsub defaults all limits to Infinity. Setting finite values provides
// defense-in-depth against resource exhaustion via crafted control messages.
// NOTE: maxIhaveMessageIDs / maxIwantMessageIDs are upstream field names but
// they actually bound the number of outer ControlIHave / ControlIWant entries,
// not nested messageIDs inside each entry.
// NOTE: maxMessages must stay high enough for current js-libp2p behavior:
// handleIWant() may serialize many RPC.Message entries into a single response RPC.
// See: Lighthouse v8.1.3 security patches for analogous rust-libp2p fixes.
decodeRpcLimits: {
maxSubscriptions: 512,
maxMessages: 5000,
maxIhaveMessageIDs: 256,
maxIwantMessageIDs: 16,
maxIdontwantMessageIDs: 16,
maxControlMessages: 256,
maxPeerInfos: 32,
},
Comment on lines +191 to +199
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better maintainability and readability, it's good practice to extract these magic numbers into named constants. This makes it easier to understand their purpose and to modify them in the future.

You could define these constants at the top of the file, for example after MAX_OUTBOUND_BUFFER_SIZE, and include the excellent rationale from the PR description as comments for each constant.

Example:

/** Well above the ~140 CL topics (global + attestation subnets + sync subnets + blob/column topics) */
const DECODE_RPC_MAX_SUBSCRIPTIONS = 512;
/** Generous for burst messages (attestations, blob sidecars) */
const DECODE_RPC_MAX_MESSAGES = 256;
// ... and so on for the other limits
Suggested change
decodeRpcLimits: {
maxSubscriptions: 512,
maxMessages: 256,
maxIhaveMessageIDs: 200,
maxIwantMessageIDs: 200,
maxIdontwantMessageIDs: 2000,
maxControlMessages: 500,
maxPeerInfos: 100,
},
decodeRpcLimits: {
maxSubscriptions: DECODE_RPC_MAX_SUBSCRIPTIONS,
maxMessages: DECODE_RPC_MAX_MESSAGES,
maxIhaveMessageIDs: DECODE_RPC_MAX_IHAVE_MESSAGE_IDS,
maxIwantMessageIDs: DECODE_RPC_MAX_IWANT_MESSAGE_IDS,
maxIdontwantMessageIDs: DECODE_RPC_MAX_IDONTWANT_MESSAGE_IDS,
maxControlMessages: DECODE_RPC_MAX_CONTROL_MESSAGES,
maxPeerInfos: DECODE_RPC_MAX_PEER_INFOS,
},

Copy link
Copy Markdown
Contributor Author

@lodekeeper lodekeeper Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a single 7-value object literal with a descriptive comment block and a rationale table in the PR body, I think the inline values are clear enough. Extracting to file-level constants would roughly double the line count without adding much readability — the values are only referenced in this one place. Keeping as-is.

})(modules.libp2p.services.components) as GossipSubInternal;

if (metrics) {
Expand Down
Loading