diff --git a/crates/inputs/inputs/src/server.rs b/crates/inputs/inputs/src/server.rs index c7aa6fa85..af2023164 100644 --- a/crates/inputs/inputs/src/server.rs +++ b/crates/inputs/inputs/src/server.rs @@ -30,7 +30,7 @@ use lightyear_connection::prelude::NetworkTarget; use lightyear_connection::server::Started; use lightyear_core::id::RemoteId; use lightyear_core::prelude::LocalTimeline; -use lightyear_core::tick::TickDuration; +use lightyear_core::tick::{Tick, TickDuration}; use lightyear_link::prelude::{LinkOf, Server}; use lightyear_messages::plugin::MessageSystems; use lightyear_messages::prelude::MessageReceiver; @@ -38,6 +38,52 @@ use lightyear_messages::server::ServerMultiMessageSender; use lightyear_replication::prelude::{PreSpawned, RoomId, Rooms}; use tracing::{debug, error, trace}; +/// Maximum number of ticks ahead of the server's current tick that an +/// incoming [`InputMessage::end_tick`] is allowed to be. Messages with +/// `end_tick > server_tick + MAX_INPUT_LOOKAHEAD_TICKS` are dropped before +/// they are written into the [`InputBuffer`]. +/// +/// Without this bound, [`InputBuffer::extend_to_range`] / [`InputBuffer::set_raw`] +/// extend the internal `VecDeque` to fit *any* tick value (filling intermediate +/// entries with `Absent` / `SameAsPrecedent`). A modified client sending +/// `end_tick = current + 30_000` would cause a 30 000-entry allocation per +/// message; repeated across messages and connections, the server is +/// memory-exhausted. +/// +/// Legitimate clients run at most a few ticks ahead of the server (typical +/// `InputDelayConfig` values are 0–3 ticks). 64 ticks (~1 s at 64 Hz) is +/// generous compared to that range while still bounding attacker memory cost +/// per message. +const MAX_INPUT_LOOKAHEAD_TICKS: i32 = 64; + +/// Maximum number of ticks *behind* the server's current tick that an incoming +/// [`InputMessage::end_tick`] is allowed to be. +/// +/// Past-direction messages are normally handled harmlessly by +/// [`InputBuffer::set_raw`]'s start-tick guard. The reason we still bound them: +/// `Tick - Tick` returns `i32` via signed wrapping arithmetic over `u32` tick +/// values. A far-future tick at the `i32` boundary +/// (`end_tick = server_tick + 2^31`) wraps to `i32::MIN`, which a naive +/// `delta <= LOOKAHEAD` check would accept. Rejecting "very far past" deltas +/// catches that wrap-around attack while still accepting reasonable late inputs +/// (up to ~4 s of network lag at 64 Hz). The wrap-around scenario is +/// astronomically improbable (2^31 ticks at 64 Hz ≈ 1.06 years of continuous +/// server runtime) but the symmetric bound is cheap belt-and-suspenders. +const MAX_INPUT_PAST_TICKS: i32 = 256; + +/// Returns `true` iff `end_tick - server_tick` falls within +/// `[-MAX_INPUT_PAST_TICKS, MAX_INPUT_LOOKAHEAD_TICKS]`. See those constants +/// for the threat model behind each bound. +/// +/// The symmetric past bound is what makes this safe under wrap-around: a naive +/// `delta <= MAX_LOOKAHEAD` check accepts `i32::MIN` because it is trivially +/// `<=` any positive bound. Rejecting on both ends eliminates the ambiguity +/// regardless of which interpretation an attacker intended. +pub(crate) fn is_input_within_lookahead(end_tick: Tick, server_tick: Tick) -> bool { + let delta = end_tick - server_tick; + (-MAX_INPUT_PAST_TICKS..=MAX_INPUT_LOOKAHEAD_TICKS).contains(&delta) +} + /// Server-side plugin that receives input messages from clients and applies /// them to [`InputBuffer`] components. /// @@ -219,6 +265,23 @@ fn receive_input_message( "server received input message" ); + // Reject messages whose end_tick is implausibly far from the + // server's current tick before any buffer write. A modified + // client sending a far-future end_tick would otherwise force + // `InputBuffer::set_raw` to allocate one entry per intermediate + // tick (memory-exhaustion DoS). See `is_input_within_lookahead`. + if !is_input_within_lookahead(message.end_tick, tick) { + trace!( + ?tick, + ?client_id, + end_tick = ?message.end_tick, + "Dropping input message: end_tick outside [server-{}, server+{}] window", + MAX_INPUT_PAST_TICKS, + MAX_INPUT_LOOKAHEAD_TICKS, + ); + return Ok(()) + } + // TODO: or should we try to store in a buffer the interpolation delay for the exact tick // that the message was intended for? #[cfg(feature = "interpolation")] @@ -592,3 +655,57 @@ fn update_action_state( // info!("Buffer length: {}", input_buffer.len()); } } + +#[cfg(test)] +mod lookahead_tests { + use super::*; + + /// Forward bound is inclusive. + #[test] + fn accepts_within_forward_bound() { + let server = Tick(1_000); + assert!(is_input_within_lookahead(server, server)); + assert!(is_input_within_lookahead( + server + MAX_INPUT_LOOKAHEAD_TICKS, + server + )); + } + + /// One tick past the forward bound is rejected. + #[test] + fn rejects_beyond_forward_bound() { + let server = Tick(1_000); + assert!(!is_input_within_lookahead( + server + (MAX_INPUT_LOOKAHEAD_TICKS + 1), + server + )); + } + + /// Reasonable late inputs (within the past bound) are accepted. + #[test] + fn accepts_within_past_bound() { + let server = Tick(1_000); + assert!(is_input_within_lookahead( + server + (-MAX_INPUT_PAST_TICKS), + server + )); + } + + /// Far-future end_tick (the DoS payload) is rejected. + #[test] + fn rejects_far_future_end_tick() { + let server = Tick(1_000); + assert!(!is_input_within_lookahead(server + 30_000, server)); + } + + /// Wrap-around attack: `end_tick = server + 2^31` makes `end - server` + /// wrap to `i32::MIN`. The symmetric past bound rejects it; a naive + /// `delta <= MAX_LOOKAHEAD` check would have accepted it. + #[test] + fn rejects_wraparound_to_i32_min() { + let server = Tick(1_000); + let wrapped = Tick(server.0.wrapping_add(1 << 31)); + assert_eq!(wrapped - server, i32::MIN); + assert!(!is_input_within_lookahead(wrapped, server)); + } +} diff --git a/crates/tests/src/client_server/input/leafwing.rs b/crates/tests/src/client_server/input/leafwing.rs index c0effd7bf..9e1d488b7 100644 --- a/crates/tests/src/client_server/input/leafwing.rs +++ b/crates/tests/src/client_server/input/leafwing.rs @@ -443,3 +443,122 @@ fn test_leafwing_input_rebroadcast() { ); } } + +/// End_tick DoS: a forged `InputMessage` with `end_tick = server_tick + +/// 30_000` causes `InputBuffer::set_raw` to allocate ~30k entries. Goes +/// end-to-end via the public `MessageSender::send::` API (no +/// internal-API hooks) and asserts the server's buffer stays bounded. See +/// `is_input_within_lookahead` in `lightyear_inputs::server` for the defense. +#[test] +fn test_input_message_with_huge_end_tick_does_not_allocate_unbounded_buffer() { + use lightyear::input::leafwing::input_message::{LeafwingSequence, LeafwingSnapshot}; + use lightyear_inputs::input_buffer::InputBuffer; + use lightyear_inputs::input_message::{ + ActionStateSequence, InputMessage, InputTarget, PerTargetData, + }; + use lightyear_inputs::prelude::InputChannel; + use lightyear_messages::prelude::MessageSender; + use lightyear_replication::prelude::ControlledBy; + + let mut stepper = ClientServerStepper::from_config(StepperConfig::with_netcode_clients(1)); + let client_of_0 = stepper.client_of(0).id(); + + let target_server_entity = stepper + .server_app + .world_mut() + .spawn(( + ActionState::::default(), + Replicate::to_clients(NetworkTarget::All), + ControlledBy { + owner: client_of_0, + lifetime: Default::default(), + }, + )) + .id(); + + // Warm-up: let the entity replicate down to the client and (if the + // target-authorization defense is also present) `ControlledByRemote` + // auto-populate, so the forged input is authorized and actually reaches + // `set_raw` — exercising the DoS path rather than being filtered first. + stepper.frame_step(5); + + let target_local = stepper + .client(0) + .get::() + .unwrap() + .entity_mapper + .get_local(target_server_entity) + .expect("target entity should be replicated to client 0"); + + // Establish a baseline server-side InputBuffer at the current tick range. + // Without this the attack just initializes the buffer at the huge tick + // (no gap to fill, no growth). + stepper.client_apps[0] + .world_mut() + .entity_mut(target_local) + .insert(InputMap::::new([( + LeafwingInput1::Jump, + KeyCode::KeyA, + )])); + stepper.frame_step(1); + stepper.client_apps[0] + .world_mut() + .resource_mut::>() + .press(KeyCode::KeyA); + stepper.frame_step(5); + + let server_tick_before = stepper.server_tick(); + let attack_end_tick = server_tick_before + 30_000; + + // The sequence content doesn't matter — only `end_tick` controls how far + // `set_raw` extends the server's buffer. + let mut sequence_buf = + InputBuffer::, LeafwingInput1>::default(); + let mut snapshot_state = ActionState::::default(); + snapshot_state.press(&LeafwingInput1::Jump); + sequence_buf.set(attack_end_tick, LeafwingSnapshot(snapshot_state)); + let sequence = LeafwingSequence::::build_from_input_buffer( + &sequence_buf, + 1, + attack_end_tick, + ) + .expect("sequence built from non-empty buffer"); + + let mut forged: InputMessage> = + InputMessage::new(attack_end_tick); + forged.inputs.push(PerTargetData { + target: InputTarget::Entity(target_local), + states: sequence, + }); + + // Send via the public MessageSender API — models exactly what a modified + // client binary could do. + { + let client_app = &mut stepper.client_apps[0]; + let mut client_entity_mut = client_app + .world_mut() + .entity_mut(stepper.client_entities[0]); + let mut sender = client_entity_mut + .get_mut::>>>() + .expect("client has a MessageSender for InputMessage"); + sender.send::(forged); + } + + // Frames for serialize → wire → deserialize → receive. + stepper.frame_step(3); + + let buffer = stepper + .server_app + .world() + .entity(target_server_entity) + .get::, LeafwingInput1>>() + .expect("server should have an InputBuffer for the target after receiving inputs"); + let buffer_len = buffer.len(); + assert!( + buffer_len < 1_000, + "DoS: server allocated {buffer_len} entries in the InputBuffer for a \ + single forged InputMessage with end_tick = server_tick + 30000. The \ + receive path does not bound the message's end_tick — see \ + `is_input_within_lookahead` in lightyear_inputs::server.", + ); +}