Spoofed InputTarget:Entity lets a modified client hijack any entity and Unbounded end_tick lets a client exhaust server memory

[mmannerm]

Here are two input validation bugs that I found during my tinkering and prompting Claude. Note, there is downstream impact related to ControlledBy now being enforced. If you want the doc and examples updated as well, let me know. Or if you prefer more compact comments on the code.

---

The bug — two related input-pipeline security gaps

1. Spoofed InputTarget::Entity lets a modified client hijack any entity

lightyear_inputs/src/server.rs::receive_input_message reads InputTarget::Entity(entity) from the wire and writes the inputs to that entity's InputBuffer with no authorization check. Note that #1361's new client-side ControlledBy filter in prepare_input_message does NOT close this gap on its own: ControlledBy is server-side state and is never replicated to clients, so the client filter only kicks in when a user manually inserts ControlledBy on the local replica — and a tampered client trivially skips the check anyway. The receive path itself must enforce the boundary.

// lightyear_inputs/src/server.rs (upstream main, paraphrased)
for data in message.inputs {
    let Some(entity) = (match data.target {
        InputTarget::Entity(entity) => Some(entity),
        InputTarget::PreSpawned(hash) => ...,
    }) else { continue };
    if let Ok(buffer) = query.get_mut(entity) {
        // ...writes data into entity's InputBuffer with no check that
        //   `entity` belongs to the sending client.
    }
}

The legitimate client-side prepare_input_message filters by With<InputMap<A>>, so a well-behaved client only emits inputs for entities it controls. A modified client bypasses this filter:

1. The client observes another player's entity via normal replication. Its SendEntityMap is populated bidirectionally — local_to_remote maps the client's local id to the server-local id, marked "already mapped" so the receive-side mapper accepts the id as-is.
2. The modified client puts an InputMap on the foreign entity's local replica (or builds the message manually) and sends InputTarget::Entity(victim_local_id).
3. SendEntityMap remaps it to victim_server_id with the "already mapped" flag.
4. Server's receive_input_message accepts it, writes to victim's InputBuffer.
5. Server's update_action_state applies it to victim's ActionState.fixed_update_state next tick.

Any server-side gameplay system running in FixedUpdate (most game logic) now sees the attacker's input on the victim's entity.

2. Unbounded end_tick lets a client exhaust server memory

InputBuffer::set_raw (called transitively from receive_input_message) extends its VecDeque to fit any tick value passed to it, filling intermediate entries with SameAsPrecedent:

// lightyear_inputs/src/input_buffer.rs
if tick > end_tick {
    for _ in 0..(tick - end_tick - 1) {
        self.buffer.push_back(Compressed::SameAsPrecedent);
    }
    self.buffer.push_back(value);
}

A modified client sending end_tick = server_tick + 30_000 triggers a 30 000-entry allocation per message; repeated across messages and connections, the server is memory-exhausted.

The fix

Two pub(crate) helpers in lightyear_inputs/src/server.rs, both wired into receive_input_message BEFORE the rebroadcast and per-target-apply paths.

is_input_target_authorized

The natural authorization map is ControlledByRemote — the relationship-target component on a peer's connection entity, auto-populated when ControlledBy { owner: peer } is added to a controlled entity. The helper checks list membership:

pub(crate) fn is_input_target_authorized(
    target_entity: Entity,
    controlled_by_remote: Option<&ControlledByRemote>,
) -> bool {
    match controlled_by_remote {
        None => false,
        Some(controlled) => controlled.collection().contains(&target_entity),
    }
}

Returns false when the peer has no ControlledByRemote at all (no controlled entities yet) or when the target isn't in the list.

is_input_within_lookahead

pub(crate) fn is_input_within_lookahead(end_tick: Tick, server_tick: Tick) -> bool {
    let delta = end_tick - server_tick;  // i32, via wrapping_diff
    delta <= MAX_INPUT_LOOKAHEAD_TICKS && delta >= -MAX_INPUT_PAST_TICKS
}

Post-#1361, Tick is u32 and Tick - Tick returns i32. The forward bound (MAX_INPUT_LOOKAHEAD_TICKS = 64) is what blocks the unbounded-allocation DOS — a forged end_tick = server_tick + 30_000 no longer fits, and the receive path drops the message before InputBuffer::set_raw can extend the buffer. The symmetric past bound (-MAX_INPUT_PAST_TICKS = -256) is cheap belt-and-suspenders: in the i32 regime, the wrap-around vector at +2^31 requires the server to run ~1.06 years of continuous ticks at 64 Hz to reach the boundary, so it's practically unreachable, but the past bound is still useful for rejecting absurdly stale messages (256 ticks at 64 Hz ≈ 4 s of network lag — legitimate clients should not be that far behind).

Wired into receive_input_message

// 1. existing early-return for is_local() + !rebroadcast
// 2. NEW: drop message if end_tick outside the lookahead window
if !is_input_within_lookahead(message.end_tick, tick) {
    trace!(...);
    return Ok(())
}
// 3. interpolation-delay update (moved here so it runs even if filter empties)
#[cfg(feature = "interpolation")]
if let Some(delay) = message.interpolation_delay { ... }
// 4. NEW: filter inputs by authorization (bypass for is_local() host-server)
if !client_id.is_local() {
    message.inputs.retain(|data| match data.target {
        InputTarget::Entity(entity) =>
            is_input_target_authorized(entity, controlled_by_remote),
        InputTarget::PreSpawned(_) => true,  // hash-based, separate concern
    });
    if message.inputs.is_empty() { return Ok(()) }
}
// 5. rebroadcast (existing — now safely operates on filtered message)
// 6. per-target apply (existing — auth check removed, filter handled it)

InputTarget::PreSpawned(hash) bypasses the authorization check at the call site. The hash is computed from spawn tick + sorted component net-IDs + optional user salt; a client that knows the convention CAN potentially forge a colliding hash for a constant user-salted PreSpawned. This is a pre-existing risk that pre-dates this patch and is out of scope here; a follow-up could extend PreSpawned with an owner: Entity or similar binding.

Why filter pre-rebroadcast, not at the per-target apply step

If the auth check ran only inside the per-target apply loop, the #[cfg(feature = "prediction")] rebroadcast block above it would forward forged inputs to all OTHER clients before the server dropped them locally. The server would effectively become a spam relay for forged input messages. Moving the check into a retain filter before the rebroadcast closes that hole — both the rebroadcast and the per-target apply operate on the already-filtered message.

Why is_local() bypass

Host-server mode runs a client in-process with the server. Local clients drive their inputs through the server's in-process state path and may not have a ControlledByRemote relationship populated the same way (since the existing early-return for is_local() && !rebroadcast_inputs already short-circuits the common host-server path). Skipping the authorization filter for is_local() is consistent with that existing exemption and matches the trust model — a host client IS the server.

Why ControlledBy rather than HasAuthority / AuthorityPeer

The filter uses ControlledByRemote (the relationship-target of ControlledBy), not the replication-authority components (HasAuthority / AuthorityPeer). These are two distinct concepts in lightyear:

• ControlledBy { owner } — semantic ownership. "This entity is owned by peer X for input/lifetime purposes." Set at spawn and rarely changes.
• HasAuthority / AuthorityPeer — dynamic simulation authority. "This peer currently simulates this entity's state and is allowed to send replication updates for it." Can transfer at runtime via GiveAuthority / RequestAuthority.

examples/distributed_authority uses both, in the way they're designed for: player entities are spawned with ControlledBy { owner: client } (inputs flow from client); ball entities use GiveAuthority to transfer simulation authority between peers (motion driven by a With<HasAuthority> filter, NOT by input messages). The two concepts intentionally decouple.

For input authorization, ControlledBy is the right map: a peer's inputs apply to their owned entities, regardless of who's currently simulating which entity.

Caveat for advanced use. If you have a use case where input authority itself needs to follow authority transfer (e.g. a vehicle multiple players take turns driving via inputs), you'll need to update ControlledBy alongside the GiveAuthority call. Most games don't need this — distributed_authority does not — but the docstring on is_input_target_authorized flags it so users hitting this pattern know what to update.

Test — TDD-first: behavioural red on bare main, then green

The integration tests were written FIRST against bare upstream/main (no fix) to confirm both vulnerabilities are reachable on the current tip, then the fix was applied to flip them green.

Unit tests (lightyear_inputs::server::patch_tests)

Six tests covering both helpers:

patch_tests::authorization_rejects_when_controlled_by_remote_is_none
patch_tests::authorization_accepts_legitimate_target_and_rejects_foreign
patch_tests::lookahead_accepts_within_forward_bound
patch_tests::lookahead_accepts_within_past_bound
patch_tests::lookahead_rejects_wraparound_far_future
patch_tests::lookahead_rejects_wraparound_at_high_server_tick

The wrap-around tests cover the i32::MAX-side boundary at both a low server tick and an upper-half-of-u32 server tick to verify the symmetric bound catches the i32 overflow at any server-tick position. While i32 wrap-around is practically unreachable in real-world server uptime (~1 year at 64 Hz), the unit tests defend the invariant in case future changes alter the tick type or arithmetic.

Integration tests (lightyear_tests/src/client_server/input/leafwing.rs)

test_input_message_with_spoofed_target_is_rejected
test_input_message_with_only_spoofed_targets_filters_to_empty
test_input_message_with_huge_end_tick_does_not_allocate_unbounded_buffer

Test 1 — spoofed-target attack. Sets up 2 clients, two server entities (one per client) with ControlledBy. On client 0, it inserts InputMap<LeafwingInput1> on BOTH local replicas: KeyJ → Jump on the victim's, KeyA → Jump on the attacker's own. Holding both keys means the legitimate prepare_input_message query emits messages for both — one with target = victim_server_entity (the attack) and one with target = attacker_server_entity (legitimate). Two assertions:

• Negative (defense): victim's server-side ActionState::pressed(&Jump) is false.
• Positive (non-overblocking): attacker's server-side ActionState::pressed(&Jump) is true.

The negative assertion catches the spoofed-target attack landing. The positive assertion catches an over-broad defense that drops all cross-client inputs.

Test 2 — empty-after-filter early-return. Sets up just the victim entity, no entity for client 0. Client 0 forges an input message targeting the victim. After server-side retain, message.inputs is empty and the early-return branch fires. Covers a code path the first test never reaches (always leaves at least one authorized entry).

Test 3 — end_tick DOS. End-to-end via public MessageSender::send::<InputChannel> (no test-only API additions). Builds an InputMessage<LeafwingSequence<LeafwingInput1>> on the client side with end_tick = server_tick + 30_000, sends it from a client that legitimately owns the target entity. After the server's receive runs, asserts that the target's InputBuffer::len() is under 1000 (without the fix, it grows to ~30 000). Models exactly what a modified client binary could do — there are no internal-API hooks.

Behavioural red (bare upstream/main, no fix)

All three integration tests fail with explicit security-failure messages:

test test_input_message_with_spoofed_target_is_rejected ... FAILED
  → "Server applied client 0's forged input to client 1's entity
     (`ActionState::pressed` shows Jump pressed)..."

test test_input_message_with_only_spoofed_targets_filters_to_empty ... FAILED
  → "spoofed-target input was applied despite client 0 controlling nothing..."

test test_input_message_with_huge_end_tick_does_not_allocate_unbounded_buffer ... FAILED
  → "DOS: server allocated 29998 entries in the InputBuffer for a single
     forged InputMessage with end_tick = server_tick + 30000..."

Green (with the fix)

$ cargo test --package lightyear_tests --lib -- test_input_message_with

running 3 tests
test test_input_message_with_huge_end_tick_does_not_allocate_unbounded_buffer ... ok
test test_input_message_with_only_spoofed_targets_filters_to_empty ... ok
test test_input_message_with_spoofed_target_is_rejected ... ok

test result: ok. 3 passed; 0 failed

$ cargo test --package lightyear_inputs --lib --features server -- patch_tests

running 6 tests
test server::patch_tests::authorization_rejects_when_controlled_by_remote_is_none ... ok
test server::patch_tests::authorization_accepts_legitimate_target_and_rejects_foreign ... ok
test server::patch_tests::lookahead_accepts_within_forward_bound ... ok
test server::patch_tests::lookahead_accepts_within_past_bound ... ok
test server::patch_tests::lookahead_rejects_wraparound_far_future ... ok
test server::patch_tests::lookahead_rejects_wraparound_at_high_server_tick ... ok

test result: ok. 6 passed; 0 failed

Full suite — no regressions

cargo test -p lightyear_tests --lib -- --test-threads=1 (serial run, avoids unrelated parallel-test flakiness): 124 passed, 1 failed. The single failure is test_server_just_pressed, a pre-existing upstream input-flow bug that also fails on bare upstream/main; not addressed by this patch.

Safety / risk

Correctness. The authorization filter operates on ControlledByRemote membership, which is already lightyear's source of truth for which peer controls which entity. The semantics match the existing prepare_input_message send-side filter (With<InputMap<A>>) — a legitimate client only emits inputs for entities it has InputMap on, and those entities are the ones marked ControlledBy { owner: client } on the server. The receive-side filter just enforces what the send-side already does for honest clients.

Past-tick rejection. Past-direction messages were previously handled harmlessly by InputBuffer::set_raw's start-tick guard. The new MAX_INPUT_PAST_TICKS = 256 bound is a security tightening — past messages > 256 ticks behind are now dropped at receive instead of being silently discarded by set_raw. 256 ticks at 64 Hz is ~4 seconds of network lag; legitimate clients should not be that far behind. If a use case needs a higher bound, the constant can be raised — happy to make it configurable via ServerInputConfig if reviewers prefer.

Host-server mode. The is_local() bypass mirrors the existing early-return at the top of receive_input_message. Host clients pass through both gates unchanged.

Authority transfer. Documented in the helper docstring: callers must reinsert ControlledBy when transferring authority. No code change to the existing authority API.

Performance. One additional component fetch per peer per tick (the Option<&ControlledByRemote> query addition). One retain pass per received InputMessage. Both are negligible.

Compatibility risk for other consumers. Zero — both helpers are pub(crate). receive_input_message is a private system; this PR changes its internal behaviour but doesn't alter any public API surface.

Behavior change for downstream users

This is the deliberate, security-positive consequence of the patch:

An input-bearing server entity now MUST have ControlledBy { owner: <client peer entity> } for the server to accept inputs targeting it.

That requirement was previously implicit (matched what examples/distributed_authority and other real games already do) but unenforced. Most existing code already conforms — anything that uses ControlledBy to express ownership at spawn is unaffected. The change bites tests and toy setups that drove inputs through entities without ControlledBy.

Test patches included in this PR for upstream tests that were previously relying on the missing enforcement:

• lightyear_tests/src/client_server/input/leafwing.rs
  — test_leafwing_input_rebroadcast, test_rebroadcast_initializes_action_state_from_buffer
• lightyear_tests/src/client_server/input/bei.rs
  — test_input_broadcasting_prediction
• lightyear_tests/src/client_server/input/native.rs
  — test_remote_client_replicated_input, test_remote_client_predicted_input,
  test_input_broadcasting_prediction, test_input_custom_rebroadcast
• lightyear_tests/src/host_server/input/native.rs
  — test_remote_client_replicated_input, test_remote_client_predicted_input
• lightyear_tests/src/client_server/prediction/rollback.rs
  — setup_stepper_for_input_rollback (covers test_input_rollback_always_mode,
  test_last_confirmed_input_multiple_clients,
  test_input_rollback_check_mode_earliest_mismatch,
  test_no_rollback_without_input_mismatches)

Each patch is a small addition: let client_of_N = stepper.client_of(N).id(); before the spawn, plus ControlledBy { owner: client_of_N, lifetime: Default::default() } in the spawn bundle. No semantic change to the tests' intent.

Docs the maintainer may want to update (intentionally not in this PR — flagging for direction):

• book/src/concepts/advanced_replication/inputs.md — should mention that input-bearing entities need ControlledBy { owner: client } for server acceptance under the new auth filter.
• The doc-comment on receive_input_message (or ServerInputPlugin) could reference is_input_target_authorized so users tracing input flow find the gate.
• examples/* — spot-checked, the input-using examples already set ControlledBy. No changes needed.

Related

This is one of several PRs sharing root-cause analysis from a downstream project that hit the input-pipeline-trust class of bugs:

• fix(sync): raise sync_objective floor to remote + 1 (#1402) #1470 (sync floor): keeps the client tick ahead of the server's read tick.
• fix(inputs): server pop wipes get_predict fallback (#1402 part 2) #1471 (server pop wipe): keeps a fallback input live for the get_predict path.
• This PR: closes the trust-boundary holes — clients can't forge inputs for entities they don't control, and can't exhaust server memory via crafted end_tick.

[mmannerm]

Should be ready for review now

[mmannerm]

Splitting this into two focused PRs, rebased onto current main (the combined branch predated the crate restructure and the Tick u16→u32 change):

• fix(inputs): bound InputMessage end_tick lookahead (DoS) #1525 — bound InputMessage end_tick lookahead (the unbounded-allocation DoS half). Purely additive.
• feat(inputs): opt-in authorize_controlled_targets helper (spoofed-target defense) #1526 — authorize InputTarget::Entity against ControlledByRemote (the spoofed-target half). Carries the ControlledBy-enforcement behavior change, hence reviewed separately.

Both are re-derived against current main with red→green tests. Closing this one in favor of the two.