Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
dispatcher — so `COMMAND COUNT` was advertising verbs Moon could not run.

### Fixed
- **Remote panic on the cluster bus: a truncated v3 gossip header killed the process.** The gossip
wire v3 (#493) appended a 40-byte `sender_master_id`, but the deserializer's length guard still
admitted any frame of at least the v2 header size so that a genuine v2 peer would still parse —
and the v3 branch then read `data[2130..2170]` unconditionally. Any frame carrying version 3 with
a length in `2130..2170` indexed past the end. The cluster bus listener hands this function
peer-supplied bytes, so a single unauthenticated 2130-byte frame to the bus port panicked the
`cluster-ctl` thread, which by policy aborts the whole server. A short v3 header is now rejected
as malformed. Seeded into `fuzz/corpus/gossip_deser` — the target was correct but had not
synthesised the 4-byte magic plus that 40-byte length window within its PR budget.
- **`CLUSTER INFO` no longer claims `cluster_enabled`, and a slotless node no longer claims health.**
Two integration assertions encoded the pre-fix behaviour and contradicted the measured oracle:
redis-server 8.6.1 reports `cluster_enabled` in `INFO` only — `CLUSTER INFO` never carries it —
Expand Down
Binary file added fuzz/corpus/gossip_deser/v2_header_exact
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
49 changes: 49 additions & 0 deletions src/cluster/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,21 @@ pub fn deserialize_gossip(data: &[u8]) -> Result<GossipMessage, String> {
// v3 appended the sender's master id. A v2 (or v1) peer sent no such field
// and its sections start 40 bytes earlier — so the header layout, not just
// the flags encoding, depends on the version.
// The length guard above admits anything >= HEADER_SIZE_V2 so a legitimate
// v2 peer parses; a v3 header cut off inside this field would otherwise
// index past the end. A real v3 sender always writes the full 40 bytes, so
// a short one is malformed and is rejected rather than zero-filled.
let mut sender_master_id = [0u8; 40];
let header_len = if version <= GOSSIP_VERSION_NO_MASTER_ID {
HEADER_SIZE_V2
} else {
if data.len() < HEADER_SIZE {
return Err(format!(
"v{version} header truncated inside sender_master_id: {} < {}",
data.len(),
HEADER_SIZE
));
}
sender_master_id.copy_from_slice(&data[HEADER_SIZE_V2..HEADER_SIZE]);
HEADER_SIZE
};
Expand Down Expand Up @@ -981,6 +992,44 @@ mod tests {

/// A v2 peer sends no master id, and must still parse — sections included.
///
/// A v3 header truncated INSIDE `sender_master_id` must be rejected, not
/// panic.
///
/// The length guard at the top of `deserialize_gossip` admits any frame of
/// at least `HEADER_SIZE_V2` (2130) bytes so a legitimate v2 peer can be
/// parsed — but the v3 branch then reads `data[2130..2170]`
/// unconditionally. Every length in `2130..2170` carrying version 3 or
/// above therefore indexes past the end. The cluster bus listener feeds
/// this function peer-supplied bytes, so the failure mode is a remote
/// panic — and `cluster-ctl` aborts the process — not a bad parse.
#[test]
fn test_deserialize_rejects_v3_header_truncated_inside_master_id() {
let msg = GossipMessage {
msg_type: GossipMsgType::Ping,
sender_node_id: [b'a'; 40],
sender_slots: Box::new([0u8; 2048]),
config_epoch: 1,
sender_ip: [
b'1', b'2', b'7', b'.', b'0', b'.', b'0', b'.', b'1', 0, 0, 0, 0, 0, 0, 0,
],
sender_port: 7000,
sender_bus_port: 17000,
sender_master_id: [b'm'; 40],
gossip_sections: vec![],
};
let full = serialize_gossip(&msg);
assert!(full.len() >= HEADER_SIZE);

// Every truncation point strictly inside the appended field.
for len in HEADER_SIZE_V2..HEADER_SIZE {
let truncated = &full[..len];
assert!(
deserialize_gossip(truncated).is_err(),
"a v3 header cut at {len} bytes must be rejected, not panic"
);
}
}

/// The field decodes as absent, which is indistinguishable from "I am a
/// master". That is exactly the pre-v3 behaviour and the reason adding it
/// required a version bump rather than a silent field append.
Expand Down
Loading