fix: remote-input DoS hardening (unbounded allocation, panics, missing limits)#514
Open
varex83agent wants to merge 13 commits into
Open
fix: remote-input DoS hardening (unbounded allocation, panics, missing limits)#514varex83agent wants to merge 13 commits into
varex83agent wants to merge 13 commits into
Conversation
A peer-controlled version string with an over-long digit run (e.g. from
peerinfo/dkg-sync over the wire) overflowed usize::parse and hit
.expect("invalid regex"), panicking the per-connection handler — a
remote-triggerable DoS. Map the parse failure to
SemVerError::InvalidFormat instead; callers already handle Err.
Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Legacy definition versions v1.0-v1.4 route an untrusted num_validators (u64 from JSON, reachable over the network) through repeat_v_addresses, an unbounded clone loop, before any length/signature check. A value up to u64::MAX drives unbounded allocation / OOM. Cap at SSZ_MAX_VALIDATORS (65536, the CompositeList[65536] bound) inside repeat_v_addresses before the loop, returning DefinitionError::NumValidatorsTooLarge. Hardening beyond Charon (whose signed int + SSZ bound implicitly guard this); it cannot reject any definition that could round-trip SSZ hashing. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
An SSZ bitlist whose final byte is 0x00 (missing the length-sentinel bit) underflowed msb to 255, then evaluated 1u8 << 255 — a debug-mode shift-overflow panic (masked to 1<<7 in release). Reachable via the public HashWalker::put_bitlist on attacker-influenced input. Guard the zero-final-byte case up front and return HasherError::InvalidBitlistDelimiter. Deliberate fail-closed divergence from fastssz (which wraps to a bogus length); a well-formed bitlist always has a non-zero final byte. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
PeerStore accumulated a Peer per closed connection in inactive_peers (never trimmed) and a peer_addresses entry per identify (never removed). On a relay (gater defaults open) a remote rotating self-asserted PeerIds grows both without bound. Cap inactive_peers at MAX_INACTIVE_PEERS=1024 with oldest-first eviction that never drops known cluster peers (tracked via a companion VecDeque), and drop peer_addresses on disconnect for non-known peers with no remaining connections. Pluto-specific structure with no Charon analogue; defensive hardening. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Every identify::Event::Received stored the peer's self-asserted listen_addrs for any peer with no count bound. On a relay a remote can flood addresses that are never used, since all peer_addresses consumers look up known cluster peers only. Gate the write behind is_known_peer (dropping unknown-peer addrs without cloning) and truncate stored addresses to MAX_PEER_ADDRESSES=32. Hardening beyond Charon (peer_addresses is Pluto-specific); the gate bounds the key space to the finite cluster set. Builds on the T04 disconnect prune. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
create_relay_config built relay::Config with a struct literal that hard-coded reservation_rate_limiters and circuit_src_rate_limiters to empty, discarding rust-libp2p's default per-peer (30/2min) and per-IP (60/min) token-bucket limiters — leaving the public relay open to reservation/circuit floods. Build from relay::Config::default() (via functional-update) to retain the four defaults, and append a per-IP reservation rate limiter sized from max_res_per_peer, mirroring Charon's relay.DefaultResources() + MaxReservationsPerIP = MaxResPerPeer. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
load_files_unordered spawned one async task per keystore file with no cap and ran the CPU/memory-heavy scrypt KDF directly on the async runtime; a directory of many keystores could spawn unbounded tasks and starve the reactor. Gate both loaders with a 10-permit Semaphore (acquired before spawn, matching Charon's loadStoreWorkers=10) and move the KDF in load_files_unordered onto spawn_blocking. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Consensus::handle verified every entry in pb_msg.justification (a secp256k1 recovery each), bounded only by the 32MB wire cap — a large message could pack many small entries into O(many) recoveries (Byzantine-insider CPU amplification). Reject before the loop when the count exceeds MAX_JUSTIFICATIONS_PER_NODE * node_count(); honest QBFT never exceeds O(node_count). Defensive hardening beyond Charon; sized as a superset of every honest justification set. Part 1 of T08. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Several clients read remote bodies with no size cap (and some no timeout), letting a hostile/slow upstream exhaust memory or stall: - obolapi http_get: stream body capped at 16MB; error bodies truncated - cluster fetch_definition: stream body capped at 16MB before decode - eth1wrap ERC-1271 call: wrap in a 10s per-call timeout - p2p bootnode relay query: client timeout + 1MB body cap - CLI mev/beacon diagnostics: shared timeout-configured client + body caps Body caps are hardening beyond Charon, each sized well above any real response so no legitimate body is rejected; timeouts are parity-aligned. Part 2 of T08. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
peerinfo's nickname and version Prometheus labels are set verbatim from peer-controlled values with no length bound (the SemVer pre-release group is unbounded, so a v1.2.3-<megabytes> version parses and is used as a label); monitoringapi's beacon_node_version label comes from the raw BN version string. Truncate all three to 64 bytes on a char boundary at the metric boundary. The peerinfo prev-nickname reset uses the same truncated form; monitoringapi still passes the full version to the compatibility check. git_hash and wordlist-based labels are already bounded. Hardening beyond Charon; truncation only affects pathological inputs. Part 3 of T08. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
iamquang95
approved these changes
Jul 3, 2026
…os-hardening # Conflicts: # crates/ssz/src/hasher.rs
quick-xml 0.39.4 is flagged by RUSTSEC for unbounded namespace allocation (fixed in >=0.41.0). Bump the workspace dependency and dedupe the lockfile to a single quick-xml 0.41.0. Co-authored-by: varex83 <ivan.uvarov02@gmail.com>
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
2 participants
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.
Summary
Implements the Remote-Input DoS Hardening plan (P0): size/count caps, fail-closed parsing, bounded concurrency, and bounded metric cardinality on attacker- or upstream-influenced input. Fixes two directly exploitable issues (a remotely-triggerable panic and an unbounded allocation) plus a set of unbounded-growth / amplification hardening items.
Each commit is one task from the plan (T08 split into its 3 natural parts). Every cap is sized as a strict superset of legitimate input, so no valid input is rejected.
What's fixed
SemVer::parseno longer.expect()-panics on integer overflow from a peer-sent version string — fails closed withInvalidFormat(callers already handleErr)corev1.0–v1.4) capnum_validatorsatSSZ_MAX_VALIDATORS(65536) before therepeat_v_addressesclone loop, killing the OOM vectorclusterparse_bitlistrejects an all-zero final byte instead of the1u8 << 255debug-panic / masked-shiftsszPeerStore.inactive_peersbounded to 1024 with oldest-first eviction (never evicting known peers);peer_addressespruned on disconnectp2pp2pvec![]) + adds a per-IP reservation cap mirroring Charon'sMaxReservationsPerIPrelay-serverloadStoreWorkers); KDF moved tospawn_blockingeth2utilParity notes
The following are hardening beyond Charon (flagged in code comments as intentional, safe divergences), each sized so it cannot reject a legitimate input:
num_validatorscap (Charon relies on its signedint+ SSZCompositeList[65536])PeerStoreis Pluto-specific, no Charon analogue)Timeouts (eth1wrap, cli diagnostics) are parity-aligned with Charon's context deadlines.
Dependency changes
cluster/p2p: added thestreamfeature toreqwest(+futuresforcluster) for streaming body-size capseth1wrap: promotedtokiofrom dev-dependency to dependency for the per-call timeoutCargo.lockupdated accordinglyTesting
cargo +nightly fmt --all --check— cleancargo clippy --workspace --all-targets --all-features -- -D warnings— cleancargo test --workspace --all-features— 1953 passed, 0 failedNew unit tests accompany each task (overflow fail-closed,
num_validatorsboundary at 65536/65537, zero-byte bitlist, eviction + known-peer exemption, address cap/gate, restored limiter counts, >10-file keystore loaders, justification cap boundary, capped-body read, label truncation).🤖 Generated with Claude Code