From 7a5aaac0dfecdffa9508e78079ad68fb8154d15d Mon Sep 17 00:00:00 2001 From: Samuel Lison Date: Thu, 30 Jul 2026 16:56:54 +1000 Subject: [PATCH] fix(combat): rate-limit bow shots + validate bow inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bow release handler enforced no cooldown — the only client-driven action in the engine without one (melee, harvesting and item use all rate-limit). Each release also costs up to MaxRange/StepSize server raycasts to simulate the arc, so the gate now runs BEFORE the arrow is spent and before the simulation. - Combat.Bow.Cooldown (0.35s) added to CombatConfig + the EngineConfig schema, so it's tunable no-code in SurvivorCore Studio. - onBowRelease honours def.weaponCooldown first, falling back to Bow.Cooldown. weaponCooldown was previously read only by the melee path, even though the authoring form offers it for every weapon — a bow cooldown was silently ignored. Its plugin label is now "Cooldown (s)" noting it covers both. - A release with no matching draw is rejected (a real client fires BowDraw on press, BowRelease on release). - BowDraw validates the sender is alive and holding a bow; it previously accepted anything from anyone. - Aim points are checked for finiteness: a non-finite Vector3 defeats magnitude comparisons and would reach the raycast after the arrow was already spent. - lastShot is cleared in PlayerRemoving alongside lastSwing/drawStart. Affects v0.8.0 and earlier. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 11 +++++++++ docs/combat.md | 9 +++++-- plugin/ContentAdmin.luau | 8 +++++- src/shared/CombatConfig.luau | 2 ++ src/shared/EngineConfig.luau | 1 + src/systems/Combat.luau | 48 +++++++++++++++++++++++++++++++++--- 6 files changed, 73 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be803e9..a79ab9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ is promoted to the new version and `main` is tagged `vX.Y.Z`. ## Unreleased +### Security +- **Bow shots are now rate-limited server-side.** The bow release handler enforced no cooldown — the + one client-driven action in the engine that didn't — so a client could fire far faster than a bow's + design rate, and each release costs the server up to `MaxRange / Bow.StepSize` raycasts to simulate + the arc. The gate runs **before** the arrow is spent and before the simulation, honours a bow's own + **`weaponCooldown`** (which previously only melee read, despite being offered for every weapon), and + falls back to the new **`Combat.Bow.Cooldown`** (0.35s, tunable in SurvivorCore Studio). A release + with no matching draw is rejected, `BowDraw` now validates the sender is alive and holding a bow, + and aim points are checked for finiteness so a malformed one can't consume an arrow. Affects + v0.8.0 and earlier. + ### Added - **Player interact window** — walk up to another player and an **"[E] Interact"** badge appears over *their* head; press **E** (or tap) to open a window with their name + survival stats and an diff --git a/docs/combat.md b/docs/combat.md index 758cc80..046a3b0 100644 --- a/docs/combat.md +++ b/docs/combat.md @@ -127,9 +127,14 @@ end) ```lua Config.override("Combat", { MeleeRange = 8, MeleeCooldown = 0.6, RequireLineOfSight = true, FriendlyFire = false, - Bow = { Gravity = 80, ProjectileSpeed = 180, MaxRange = 300, MinDrawDamageMult = 0.3, StepSize = 4 }, + Bow = { + Gravity = 80, ProjectileSpeed = 180, MaxRange = 300, MinDrawDamageMult = 0.3, StepSize = 4, + Cooldown = 0.35, -- seconds between accepted shots (fallback when a bow sets no weaponCooldown) + }, }) ``` -Per-weapon `weapon*` values override these fallbacks. **Out of scope:** durability, blocking/parrying, +Per-weapon `weapon*` values override these fallbacks — including **`weaponCooldown`, which governs +both melee swings and bow shots**. Every shot is rate-limited server-side before any arrow is spent +or any arc is simulated, so a client can't out-run its own fire rate. **Out of scope:** durability, blocking/parrying, and AoE are creator content via the hooks above; mob AI lives in [mobs.md](mobs.md). diff --git a/plugin/ContentAdmin.luau b/plugin/ContentAdmin.luau index a548d4d..ad0b300 100644 --- a/plugin/ContentAdmin.luau +++ b/plugin/ContentAdmin.luau @@ -64,7 +64,13 @@ ContentAdmin.CATEGORIES = { { attr = "weaponKind", kind = "string", label = "Kind", default = "melee", placeholder = "melee / bow" }, { attr = "weaponDamage", kind = "number", label = "Damage", default = 10 }, { attr = "weaponRange", kind = "number", label = "Range (melee)", default = 8 }, - { attr = "weaponCooldown", kind = "number", label = "Cooldown", default = 0.6 }, + { + attr = "weaponCooldown", + kind = "number", + label = "Cooldown (s)", + default = 0.6, + placeholder = "melee swings AND bow shots", + }, { attr = "weaponDrawTime", kind = "number", label = "Draw time (bow)", default = 1 }, { attr = "weaponProjectileSpeed", kind = "number", label = "Arrow speed (bow)", default = 180 }, { diff --git a/src/shared/CombatConfig.luau b/src/shared/CombatConfig.luau index f4bc00a..8c1eca5 100644 --- a/src/shared/CombatConfig.luau +++ b/src/shared/CombatConfig.luau @@ -26,6 +26,8 @@ CombatConfig.DEFAULTS = { MaxRange = 300, -- studs the server simulates a projectile before giving up MinDrawDamageMult = 0.3, -- damage multiplier at zero draw; scales up to 1.0 at full draw StepSize = 4, -- studs per raycast step when simulating the arc (smaller = more precise, costlier) + Cooldown = 0.35, -- seconds between accepted shots per player (fallback when a bow def + -- sets no weaponCooldown). Rate-limits BOTH the damage and the arc simulation's raycasts. }, } diff --git a/src/shared/EngineConfig.luau b/src/shared/EngineConfig.luau index 2f35fec..a4c3e1b 100644 --- a/src/shared/EngineConfig.luau +++ b/src/shared/EngineConfig.luau @@ -161,6 +161,7 @@ EngineConfig.SECTIONS = { num("MaxRange", "Max range (studs)", 1), num("MinDrawDamageMult", "Min-draw damage mult (0-1)", 0, 1), num("StepSize", "Sim step size (studs)", 1), + num("Cooldown", "Shot cooldown (s)", 0), }, }, }, diff --git a/src/systems/Combat.luau b/src/systems/Combat.luau index 200a33c..3485e05 100644 --- a/src/systems/Combat.luau +++ b/src/systems/Combat.luau @@ -40,6 +40,20 @@ local Combat = {} local started = false local lastSwing: { [Player]: number } = {} local drawStart: { [Player]: number } = {} +local lastShot: { [Player]: number } = {} -- bow release rate limit (see onBowRelease) + +-- A client-supplied aim point must be a real, finite, sanely-bounded position. `typeof == "Vector3"` +-- alone is not enough: a NaN/inf component defeats magnitude comparisons (every comparison against +-- NaN is false), which would poison the arc simulation's raycasts. +local MAX_AIM_MAGNITUDE = 1e6 +local function isSanePoint(p: Vector3): boolean + local x, y, z = p.X, p.Y, p.Z + -- NaN never equals itself; the abs() bound rejects ±inf and absurd coordinates alike. + if x ~= x or y ~= y or z ~= z then + return false + end + return math.abs(x) < MAX_AIM_MAGNITUDE and math.abs(y) < MAX_AIM_MAGNITUDE and math.abs(z) < MAX_AIM_MAGNITUDE +end local function playerRoot(player: Player): BasePart? local char = player.Character @@ -270,7 +284,7 @@ end -- server recomputes the true fire direction from the bow's own origin, eliminating shoulder-camera -- parallax and any client direction spoofing. local function onBowRelease(player: Player, targetPoint: any, _clientAlpha: any) - if not isAlive(player) or typeof(targetPoint) ~= "Vector3" then + if not isAlive(player) or typeof(targetPoint) ~= "Vector3" or not isSanePoint(targetPoint) then drawStart[player] = nil return end @@ -285,10 +299,32 @@ local function onBowRelease(player: Player, targetPoint: any, _clientAlpha: any) return end - -- Server-timed draw (anti-cheat): how long the player actually held, clamped to the draw time. local cfg = CombatConfig.get() + + -- Rate limit, mirroring the melee path. This gate sits BEFORE the ammo spend and before + -- simulateArrow on purpose: a release costs up to MaxRange/StepSize server raycasts, so an + -- ungated handler lets one client burn the server's heartbeat regardless of ammo — and an + -- ammo-free bow (weaponAmmo = "", the authoring default) would otherwise have no limit at all. + -- Honours a bow's own weaponCooldown, which until now only the melee path read. + local cooldown = tonumber(def.weaponCooldown) or tonumber(cfg.Bow.Cooldown) or 0.35 + local now = os.clock() + if lastShot[player] and now - lastShot[player] < cooldown then + drawStart[player] = nil + return + end + + -- A release with no matching draw is not a real shot: the client fires BowDraw on press and + -- BowRelease on release. Without this, a spammed release still lands damage at the + -- MinDrawDamageMult floor without ever paying the draw time. + if not drawStart[player] then + return + end + + lastShot[player] = now + + -- Server-timed draw (anti-cheat): how long the player actually held, clamped to the draw time. local drawTime = math.max(0.01, tonumber(def.weaponDrawTime) or 1) - local held = drawStart[player] and (os.clock() - drawStart[player]) or 0 + local held = os.clock() - drawStart[player] drawStart[player] = nil local alpha = math.clamp(held / drawTime, 0, 1) @@ -350,6 +386,11 @@ function Combat.start(_options: { [string]: any }?) onMeleeSwing(player) end) Remotes.event("BowDraw").OnServerEvent:Connect(function(player) + -- Validated like every other client-driven entry point: only a living player holding a bow + -- may open a draw (the timestamp feeds the release's damage scaling). + if not isAlive(player) or not equippedWeapon(player, "bow") then + return + end drawStart[player] = os.clock() end) Remotes.event("BowRelease").OnServerEvent:Connect(function(player, targetPoint, clientAlpha) @@ -359,6 +400,7 @@ function Combat.start(_options: { [string]: any }?) Players.PlayerRemoving:Connect(function(player) lastSwing[player] = nil drawStart[player] = nil + lastShot[player] = nil end) end