Is this code secure from exploit?

Help and Feedback Code Review
,
#1

I’m working on code for a combat round system game but I’m uncertain if the code I’m using is unexploitable or not.

My concerns are mainly inside of the Supernova and Meteor signal where hitbox detection is being done. Is this valid?

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TweenService = game:GetService("TweenService")
local RunService = game:GetService("RunService")
local Players = game:GetService("Players")

local Remotes = ReplicatedStorage.Remotes
local Assets = ReplicatedStorage.Assets
local Sounds = Assets.Sounds
local Decals = Assets.Decals
local Children = Decals:GetChildren()

local Player = Players.LocalPlayer
local Character = Player.Character or Player.CharacterAdded:Wait()
local Root = Character:WaitForChild("HumanoidRootPart")
local Humanoid = Character:WaitForChild("Humanoid")

local Info = TweenInfo.new(10,Enum.EasingStyle.Linear,Enum.EasingDirection.In)

Remotes.ToClient.OnClientEvent:Connect(function(signal,targetPart,sn,dir,rootPos)
	if signal == "Splat" then
		local random = Children[math.random(1,#Children)]:Clone()
		local poop = workspace:FindFirstChild(sn)
		local params = RaycastParams.new()
		params.FilterDescendantsInstances = {Character,poop}
		params.FilterType = Enum.RaycastFilterType.Exclude
		local result = workspace:Raycast(rootPos,dir,params)
		if result and result.Instance and result.Instance == targetPart then
			local splat = Assets.Splatter:Clone()
			random.Face = Enum.NormalId.Front
			random.Parent = splat
			local v1 = math.ceil(result.Normal.X)
			local v2 = math.ceil(result.Normal.Y)
			local v3 = math.ceil(result.Normal.Z)
			
			
			
			splat.CFrame = CFrame.lookAt(result.Position - (result.Normal * 2),result.Position)
			if (splat.Position - rootPos).Magnitude < 5 then
				splat:Destroy()
				return
			end
			splat.Parent = workspace
			local tween = TweenService:Create(random,Info,{Transparency = 1})
			tween:Play()
			tween.Completed:Connect(function()
				splat:Destroy()
			end)
		end
	elseif signal == "Explode" then
		local part = workspace:FindFirstChild(targetPart)
		if not part then return end
		local inf1 = TweenInfo.new(0.5,Enum.EasingStyle.Bounce,Enum.EasingDirection.Out)
		local inf2 = TweenInfo.new(0.5,Enum.EasingStyle.Sine,Enum.EasingDirection.Out)
		local inf3 = TweenInfo.new(1,Enum.EasingStyle.Sine,Enum.EasingDirection.Out)
		TweenService:Create(part,inf1,{Size = Vector3.one * 8}):Play()
		TweenService:Create(part,inf2,{Transparency = 1}):Play()
	elseif signal == "PlayerIsSuckedIn" then
		local poop = workspace:FindFirstChild(sn)
		local ap = Instance.new("AlignPosition")
		ap.Mode = Enum.PositionAlignmentMode.OneAttachment
		ap.MaxVelocity = 1e6
		ap.MaxForce = 1e6
		ap.Responsiveness = 100
		ap.Attachment0 = targetPart.HumanoidRootPart.RootAttachment
		ap.Parent = targetPart
		task.spawn(function()
			while true do
				ap.Position = poop.Position
				task.wait()
				if poop.Parent ~= workspace then
					ap:Destroy()
					break
				end
			end
		end)
	elseif signal == "Supernova" then
		local inf1 = TweenInfo.new(0.5,Enum.EasingStyle.Exponential,Enum.EasingDirection.Out)
		local inf2 = TweenInfo.new(2,Enum.EasingStyle.Exponential,Enum.EasingDirection.Out)
		
		local spn = Sounds.Supernova:Clone()
		local sTouched = false
		
		local c = BrickColor.random()
		local super = Assets.Supernova:Clone()
		
		local function t(hit)
			local character = hit.Parent
			if character ==  nil then return end
			local humanoid = character:FindFirstChild("Humanoid")
			if humanoid and humanoid == Humanoid and not sTouched then
				sTouched = true
				Remotes.ToServer:FireServer("TouchedSupernova",super.PrimaryPart.Position)
			end
		end
		
		super.Parent = workspace
		super:MoveTo(targetPart)
		for i,v in super:GetChildren() do
			v.BrickColor = c
			v.Touched:Connect(t)
		end
		
		spn:Play()
		spn.Parent = super.Star
		task.delay(spn.TimeLength,spn.Destroy,spn)
		
		TweenService:Create(super.Impact,inf1,{Size = Vector3.new(0.5,20,20)}):Play()
		TweenService:Create(super.Star,inf1,{Size = Vector3.one * 17.5}):Play()
		
		TweenService:Create(super.Impact,inf2,{Transparency = 1}):Play()
		TweenService:Create(super.Star,inf2,{Transparency = 1}):Play()
		
		task.wait(2)
		super:Destroy()
	elseif signal == "Fire" then
		local part = workspace:FindFirstChild(targetPart)
		local fire = Assets.Fire:Clone()
		fire.Parent = part
	elseif signal == "PlayerFire" then
		local fire = Assets.Fire:Clone()
		local f = Sounds.Fire:Clone()
		f.TimePosition = 1
		f:Play()
		f.Parent = targetPart
		fire.Parent = targetPart
		task.delay(3,f.Destroy,f)
		task.delay(3,fire.Destroy,fire)
	elseif signal == "Meteor" then
		local meteor = Assets.Meteor:Clone()
		local sfx_wind = Sounds.Meteor.Wind:Clone()
		local sfx_impact = Sounds.Meteor.Impact:Clone()
		local impact = meteor.Impact
		local rock = meteor.Rock
		local fire = rock.Attachment.ParticleEmitter
		local ray = targetPart
		local originP = sn
		local targetP = dir
		local touched = false
		local c
		
		sfx_wind:Play()
		sfx_wind.Parent = rock
		sfx_impact.Parent = impact
		
		meteor.Parent = workspace
		meteor:MoveTo(originP)
		
		impact.Touched:Connect(function(hit)
			local character = hit.Parent
			if character == Character and not touched then
				touched = true
				Remotes.ToServer:FireServer("PlayerWasHitByMeteor",impact.Position)
			end
		end)
		
		local moveRock = TweenService:Create(rock,TweenInfo.new(0.75,Enum.EasingStyle.Linear,Enum.EasingDirection.InOut),{Position = targetP + Vector3.new(0,rock.Size.Y/2,0)})
		
		c = RunService.Heartbeat:Connect(function()
			impact.Position = rock.Position
		end)
		
		moveRock:Play()
		moveRock.Completed:Connect(function()
			rock:Destroy()
			c:Disconnect()
			impact.Anchored = true
			local impactBurst = TweenService:Create(impact,TweenInfo.new(0.75,Enum.EasingStyle.Exponential,Enum.EasingDirection.Out),{Size = impact.Size + Vector3.one * 10})
			impactBurst:Play()
			sfx_impact:Play()
			impactBurst.Completed:Connect(function()
				meteor:Destroy()
			end)
		end)
	end
end)
2 Likes
#2

both are vulnerable just by repeatedly firing your remote on everyone’s position (assuming you don’t have checks on the derver and the signals instakill). There’s probably more but this is the most major one I’ve noticed.

1 Like
#3

You mean sending the position to the server is bad?

I used “typeof” to check if the argument was a position and I checked the magnitude of the position as well.

1 Like
#4

Sending the position to the server isn’t that bad but you must do server-side checks to validate the position and prevent abuse through repeated or spoofed requests.
what @SownMoney707431 recommends is implementing rate limits to prevent abuse from repeated remote event calls which is important

1 Like
#5

I showed that I made checks to validate signals from the client.

1 Like
#6

You don’t

Remotes.FromClient.OnServerEvent:Connect(function(player, action, position)
	if action == "TouchedSupernova" then
		if typeof(position) == "Vector3" then
			local character = player.Character
			if character and character:FindFirstChild("HumanoidRootPart") then
				local distance = (character.HumanoidRootPart.Position - position).Magnitude
				if distance < VALID_DISTANCE_THRESHOLD then
					-- damage or effects
				end
			end
		end
		--rate limit here
	end
end)

In this part your code only validates signals from the client but doesn’t secure them because it lacks server-side checks so that’s why you should rate limit

1 Like
#7

I haven’t shown the server script yet. Up there is the client script which is completely vulnerable to the client. I just said what I did though.

1 Like