Skip to content

Commit

Permalink
Rough click slot packet validation (#293)
Browse files Browse the repository at this point in the history
## Description

This adds some validation for incoming inventory packets that makes it
so that you can't just spawn items by sending malicious packets. It adds
type 1 and type 2 validations as outlined in #292.

This also adds some new helpers, `InventoryWindow` and
`InventoryWindowMut`.

fixes #292

<details>

<summary>Playground</summary>

```rust
use valence::client::{default_event_handler, despawn_disconnected_clients};
use valence::prelude::event::PlayerInteractBlock;
use valence::prelude::*;

#[allow(unused_imports)]
use crate::extras::*;

const SPAWN_Y: i32 = 64;
const CHEST_POS: [i32; 3] = [0, SPAWN_Y + 1, 3];

pub fn build_app(app: &mut App) {
    app.add_plugin(ServerPlugin::new(()).with_connection_mode(ConnectionMode::Offline))
        .add_startup_system(setup)
        .add_system(default_event_handler.in_schedule(EventLoopSchedule))
        .add_system(init_clients)
        .add_system(despawn_disconnected_clients)
        .add_systems((toggle_gamemode_on_sneak, open_chest).in_schedule(EventLoopSchedule));
}

fn setup(mut commands: Commands, server: Res<Server>) {
    let mut instance = server.new_instance(DimensionId::default());

    for z in -5..5 {
        for x in -5..5 {
            instance.insert_chunk([x, z], Chunk::default());
        }
    }

    for z in -25..25 {
        for x in -25..25 {
            instance.set_block([x, SPAWN_Y, z], BlockState::GRASS_BLOCK);
        }
    }
    instance.set_block(CHEST_POS, BlockState::CHEST);

    commands.spawn(instance);

    let mut inventory = Inventory::new(InventoryKind::Generic9x3);
    inventory.set_slot(0, ItemStack::new(ItemKind::Apple, 100, None));
    inventory.set_slot(1, ItemStack::new(ItemKind::Diamond, 40, None));
    inventory.set_slot(2, ItemStack::new(ItemKind::Diamond, 30, None));
    commands.spawn(inventory);
}

fn init_clients(
    mut clients: Query<(&mut Position, &mut Location, &mut Inventory), Added<Client>>,
    instances: Query<Entity, With<Instance>>,
) {
    for (mut pos, mut loc, mut inv) in &mut clients {
        pos.0 = [0.5, SPAWN_Y as f64 + 1.0, 0.5].into();
        loc.0 = instances.single();

        inv.set_slot(24, ItemStack::new(ItemKind::Apple, 100, None));
        inv.set_slot(25, ItemStack::new(ItemKind::Apple, 10, None));
    }
}

// Add new systems here!

fn open_chest(
    mut commands: Commands,
    inventories: Query<Entity, (With<Inventory>, Without<Client>)>,
    mut events: EventReader<PlayerInteractBlock>,
) {
    let Ok(inventory) = inventories.get_single() else {
        return;
    };

    for event in events.iter() {
        if event.position != CHEST_POS.into() {
            continue;
        }
        let open_inventory = OpenInventory::new(inventory);
        commands.entity(event.client).insert(open_inventory);
    }
}

```

</details>

## Test Plan

Steps:
1. `cargo test`

---------

Co-authored-by: Ryan Johnson <[email protected]>
  • Loading branch information
dyc3 and rj00a authored Mar 24, 2023
1 parent 8d93dde commit a578009
Show file tree
Hide file tree
Showing 7 changed files with 1,144 additions and 20 deletions.
82 changes: 74 additions & 8 deletions crates/valence/src/client/event.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::cmp;

use anyhow::bail;
Expand All @@ -8,7 +9,7 @@ use bevy_ecs::schedule::ScheduleLabel;
use bevy_ecs::system::{SystemParam, SystemState};
use glam::{DVec3, Vec3};
use paste::paste;
use tracing::warn;
use tracing::{debug, warn};
use uuid::Uuid;
use valence_protocol::block_pos::BlockPos;
use valence_protocol::ident::Ident;
Expand All @@ -27,15 +28,19 @@ use valence_protocol::packet::c2s::play::update_structure_block::{
use valence_protocol::packet::c2s::play::{
AdvancementTabC2s, ClientStatusC2s, ResourcePackStatusC2s, UpdatePlayerAbilitiesC2s,
};
use valence_protocol::packet::s2c::play::InventoryS2c;
use valence_protocol::packet::C2sPlayPacket;
use valence_protocol::types::{Difficulty, Direction, Hand};
use valence_protocol::var_int::VarInt;

use super::{
CursorItem, KeepaliveState, PlayerActionSequence, PlayerInventoryState, TeleportState,
};
use crate::client::Client;
use crate::component::{Look, OnGround, Ping, Position};
use crate::inventory::Inventory;
use crate::inventory::{Inventory, InventorySettings};
use crate::packet::WritePacket;
use crate::prelude::OpenInventory;

#[derive(Clone, Debug)]
pub struct QueryBlockNbt {
Expand Down Expand Up @@ -671,6 +676,7 @@ pub(crate) struct EventLoopQuery {
keepalive_state: &'static mut KeepaliveState,
cursor_item: &'static mut CursorItem,
inventory: &'static mut Inventory,
open_inventory: Option<&'static mut OpenInventory>,
position: &'static mut Position,
look: &'static mut Look,
on_ground: &'static mut OnGround,
Expand All @@ -682,10 +688,17 @@ pub(crate) struct EventLoopQuery {
/// An exclusive system for running the event loop schedule.
fn run_event_loop(
world: &mut World,
state: &mut SystemState<(Query<EventLoopQuery>, ClientEvents, Commands)>,
state: &mut SystemState<(
Query<EventLoopQuery>,
ClientEvents,
Commands,
Query<&Inventory, Without<Client>>,
Res<InventorySettings>,
)>,
mut clients_to_check: Local<Vec<Entity>>,
) {
let (mut clients, mut events, mut commands) = state.get_mut(world);
let (mut clients, mut events, mut commands, mut inventories, inventory_settings) =
state.get_mut(world);

update_all_event_buffers(&mut events);

Expand All @@ -703,7 +716,7 @@ fn run_event_loop(

q.client.dec.queue_bytes(bytes);

match handle_one_packet(&mut q, &mut events) {
match handle_one_packet(&mut q, &mut events, &mut inventories, &inventory_settings) {
Ok(had_packet) => {
if had_packet {
// We decoded one packet, but there might be more.
Expand All @@ -723,15 +736,16 @@ fn run_event_loop(
while !clients_to_check.is_empty() {
world.run_schedule(EventLoopSchedule);

let (mut clients, mut events, mut commands) = state.get_mut(world);
let (mut clients, mut events, mut commands, mut inventories, inventory_settings) =
state.get_mut(world);

clients_to_check.retain(|&entity| {
let Ok(mut q) = clients.get_mut(entity) else {
// Client must have been deleted during the last run of the schedule.
return false;
};

match handle_one_packet(&mut q, &mut events) {
match handle_one_packet(&mut q, &mut events, &mut inventories, &inventory_settings) {
Ok(had_packet) => had_packet,
Err(e) => {
warn!("failed to dispatch events for client {:?}: {e:?}", q.entity);
Expand All @@ -748,6 +762,8 @@ fn run_event_loop(
fn handle_one_packet(
q: &mut EventLoopQueryItem,
events: &mut ClientEvents,
inventories: &mut Query<&Inventory, Without<Client>>,
inventory_settings: &Res<InventorySettings>,
) -> anyhow::Result<bool> {
let Some(pkt) = q.client.dec.try_next_packet::<C2sPlayPacket>()? else {
// No packets to decode.
Expand Down Expand Up @@ -847,7 +863,57 @@ fn handle_one_packet(
});
}
C2sPlayPacket::ClickSlotC2s(p) => {
if p.slot_idx < 0 {
let open_inv = q
.open_inventory
.as_ref()
.and_then(|open| inventories.get_mut(open.entity).ok());
if let Err(msg) =
crate::inventory::validate_click_slot_impossible(&p, &q.inventory, open_inv)
{
debug!(
"client {:#?} invalid click slot packet: \"{}\" {:#?}",
q.entity, msg, p
);
let inventory = open_inv.unwrap_or(&q.inventory);
q.client.write_packet(&InventoryS2c {
window_id: if open_inv.is_some() {
q.player_inventory_state.window_id
} else {
0
},
state_id: VarInt(q.player_inventory_state.state_id.0),
slots: Cow::Borrowed(inventory.slot_slice()),
carried_item: Cow::Borrowed(&q.cursor_item.0),
});
return Ok(true);
}
if inventory_settings.enable_item_dupe_check {
if let Err(msg) = crate::inventory::validate_click_slot_item_duplication(
&p,
&q.inventory,
open_inv,
&q.cursor_item,
) {
debug!(
"client {:#?} click slot packet tried to incorrectly modify items: \"{}\" \
{:#?}",
q.entity, msg, p
);
let inventory = open_inv.unwrap_or(&q.inventory);
q.client.write_packet(&InventoryS2c {
window_id: if open_inv.is_some() {
q.player_inventory_state.window_id
} else {
0
},
state_id: VarInt(q.player_inventory_state.state_id.0),
slots: Cow::Borrowed(inventory.slot_slice()),
carried_item: Cow::Borrowed(&q.cursor_item.0),
});
return Ok(true);
}
}
if p.slot_idx < 0 && p.mode == ClickMode::Click {
if let Some(stack) = q.cursor_item.0.take() {
events.2.drop_item_stack.send(DropItemStack {
client: entity,
Expand Down
Loading

0 comments on commit a578009

Please sign in to comment.