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
119 changes: 118 additions & 1 deletion crates/inputs/inputs/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,60 @@ 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;
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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's keep this for now, but I don't think there is any point of making Tick wrapping now that it's a u32; I should just make it a normal u32.

/// 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.
///
Expand Down Expand Up @@ -219,6 +265,23 @@ fn receive_input_message<S: ActionStateSequence>(
"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")]
Expand Down Expand Up @@ -592,3 +655,57 @@ fn update_action_state<S: ActionStateSequence>(
// 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));
}
}
119 changes: 119 additions & 0 deletions crates/tests/src/client_server/input/leafwing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<InputChannel>` 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::<LeafwingInput1>::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::<MessageManager>()
.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::<LeafwingInput1>::new([(
LeafwingInput1::Jump,
KeyCode::KeyA,
)]));
stepper.frame_step(1);
stepper.client_apps[0]
.world_mut()
.resource_mut::<ButtonInput<KeyCode>>()
.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::<LeafwingSnapshot<LeafwingInput1>, LeafwingInput1>::default();
let mut snapshot_state = ActionState::<LeafwingInput1>::default();
snapshot_state.press(&LeafwingInput1::Jump);
sequence_buf.set(attack_end_tick, LeafwingSnapshot(snapshot_state));
let sequence = LeafwingSequence::<LeafwingInput1>::build_from_input_buffer(
&sequence_buf,
1,
attack_end_tick,
)
.expect("sequence built from non-empty buffer");

let mut forged: InputMessage<LeafwingSequence<LeafwingInput1>> =
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::<MessageSender<InputMessage<LeafwingSequence<LeafwingInput1>>>>()
.expect("client has a MessageSender for InputMessage<LeafwingSequence>");
sender.send::<InputChannel>(forged);
}

// Frames for serialize → wire → deserialize → receive.
stepper.frame_step(3);

let buffer = stepper
.server_app
.world()
.entity(target_server_entity)
.get::<InputBuffer<LeafwingSnapshot<LeafwingInput1>, 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.",
);
}