Is this secured enough?

Help and Feedback Code Review
#1

So I made a tool with security which has local script firing a remote event and the server script getting the remote event. After the server script checks if the player that fired it is the tool’s parent, and then changing an initial value to 2. Lastly, another server script is fired when the initial value is changed to 2.

-- Local Script
local Tool = script.Parent
local debounce = false

Tool.Activated:Connect(function()
	if not debounce then
		debounce = true
		Tool.FireEvent1:FireServer()
		wait(.5)
		debounce = false
	end
end)

-- Server Script 1
local Tool = script.Parent

Tool.FireEvent1.OnServerEvent:Connect(function(player)
	if player == game.Players:GetPlayerFromCharacter(Tool.Parent) then
		Tool.DoubleCheck.Value = 2
		wait(.5)
		Tool.DoubleCheck.Value = 0
	end
end)

-- Server Script 2 (Double Check)
local Tool = script.Parent

Tool.DoubleCheck.Changed:Connect(function()
	if Tool.DoubleCheck.Value == 2 then
		print("check")
	end
end)
#2

I would personally recommend you to store Remote Events inside ReplicatedStorage and Server Scripts inside ServerScriptService rather than having everything inside the tool, though it’s more of an organization (I would argue having the Server Script hidden from the client end is also a small bonus on security).

Instead of doing the value checking thing, you could instead just use a boolean and check true/false.

Security-wise, it ain’t bad, though I’d be careful about punishing players that fail the check.

#3

I would not punish unless I know what they are trying to do. But would this work with getting the player’s mouse hit position and other stuff?

#4

It should work with the mouse position, if anything you could pass it in through the remote event.

Though I would be cautious of clients sending info to the server as anti-cheat, as it can be spoofed (and likely pretty easily).

1 Like
#5

You need a debounce. Have another value along the lines of DoubleCheckActive and set it to true false to match the state of this event happening.

#6

The debounce on the client can be exploited, have the debounce on the server instead. Also, server scripts should usually be placed in ServerScriptService for organization and no, there is no “security” bonus involved when you place them in ServerScriptService because the bytecode or source of a server script is not replicated to the client even if it is placed where the client can access it.

If bytecode is not replicated but the source of the script is, the exploiter would just see a blank script.

-- server script
local debounce 

Tool.FireEvent1.OnServerEvent:Connect(function(player)
	if  player ~= game.Players:GetPlayerFromCharacter(Tool.Parent) or  debounce ~= nil and os.time() - debounce < .5 then return end 
        debounce = os.time()  
		Tool.DoubleCheck.Value = 2
		wait(.5)
		Tool.DoubleCheck.Value = 0
end)
#7

I would argue having the Server Script hidden from the client end is also a small bonus on security).

The bytecode of source of a server script is never replicated to the client even if it is placed somewhere where the client can see it.

2 Likes
#8

Could I just hide one server script since one is to check if the player who fired the event is the tool’s parent.

#9

I double with the users here, part deletion replicates if it is in your character, that’s why some people can have block heads, remove their limbs, remove their humanoid, or anything else, and everyone will see it. You really should put anticheats anywhere other than directly in the player’s character

#10

There is no point, as I said earlier:

The bytecode of source of a server script is never replicated to the client even if it is placed somewhere where the client can see it.

However it is good practice that you handle all your tools in 1 server script placed in ServerScriptService and you can split in modules for organization and efficiency purposes.