Merge pull request #92 from TemujinCalidius/fix/bow-rate-limit

fix(combat): rate-limit bow shots + validate bow inputs

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

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).

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 },
 			{

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.
 	},
 }
 

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),
 				},
 			},
 		},

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