harden server — startup, shutdown, overflow guards - #92
Merged
Conversation
the recursive check/parse functions had no depth limit, allowing a malicious client to stack-overflow the server with deeply nested arrays or maps. adds a MAX_NESTING_DEPTH of 64 and threads a depth counter through both check() and parse(). returns NestingTooDeep error when exceeded.
expire replay computed (seconds * 1000) as i64 which silently wrapped for very large u64 values, and pexpire cast u64 directly to i64 which sign-corrupted values above i64::MAX. both now clamp to i64::MAX.
DropHandle::spawn() used .expect() which panics if the OS can't create the thread. now logs a warning and returns a handle that falls back to inline dropping via the existing try_send Disconnected handling.
SlotRange::new() only checked invariants via debug_assert!, which is stripped in release builds. adds try_new() with runtime validation and uses it in decode_member_info() where data comes from the network. internal callers with known-valid ranges keep using new().
kacy
force-pushed
the
harden-server-startup-shutdown
branch
from
February 12, 2026 00:16
e586853 to
d7da609
Compare
kacy
added a commit
that referenced
this pull request
Feb 19, 2026
* fix: add nesting depth limit to RESP3 parser the recursive check/parse functions had no depth limit, allowing a malicious client to stack-overflow the server with deeply nested arrays or maps. adds a MAX_NESTING_DEPTH of 64 and threads a depth counter through both check() and parse(). returns NestingTooDeep error when exceeded. * fix: clamp ttl arithmetic in aof recovery to prevent overflow expire replay computed (seconds * 1000) as i64 which silently wrapped for very large u64 values, and pexpire cast u64 directly to i64 which sign-corrupted values above i64::MAX. both now clamp to i64::MAX. * fix: graceful degradation when drop thread fails to spawn DropHandle::spawn() used .expect() which panics if the OS can't create the thread. now logs a warning and returns a handle that falls back to inline dropping via the existing try_send Disconnected handling. * fix: validate slot ranges at runtime in gossip decode path SlotRange::new() only checked invariants via debug_assert!, which is stripped in release builds. adds try_new() with runtime validation and uses it in decode_member_info() where data comes from the network. internal callers with known-valid ranges keep using new(). * chore: cargo fmt
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.
summary
two rounds of full codebase audits surfaced 9 hardening issues across 12 files. this PR fixes all of them:
round 1 — server lifecycle & arithmetic
.expect()on address parsing with gracefuleprintln+exit(1)(main.rs, 3 sites)Duration::as_millis()u128→i64 cast toi64::MAXin shard TTL fields (shard.rs, 3 sites)checked_addat startup,saturating_addin internal paths, fix hardcoded10000incluster_meetto use configuredgossip_port_offset(main.rs, cluster.rs, topology.rs)round 2 — protocol, persistence, core, cluster
MAX_NESTING_DEPTH(64) to RESP3 parser to prevent stack overflow from deeply nested arrays/maps (parse.rs, error.rs)(seconds * 1000) as i64→saturating_mul+min(i64::MAX), andmilliseconds as i64→min(i64::MAX)(recovery.rs)DropHandle::spawn().expect()with graceful degradation — logs warning and falls back to inline dropping via existingtry_sendDisconnected handling (dropper.rs)SlotRange::try_new()with runtime validation, use it in gossip decode path where slot data comes from the network (slots.rs, message.rs)what was tested
cargo clippy --workspace -- -D warnings— cleancargo fmt --all— cleandesign considerations
try_send/TrySendError::Disconnectedpaths handle it naturallySlotRange::try_new()is separate fromnew()so internal callers with known-valid ranges don't pay for error handling