I am trying to make my game have sanity checks for see if an remote event or function is firing like extremely fast or something like that, but i cant find a way.
2 Likes
Any kind of check (sanity, security, …) for a remote should be added write below the event listener. Use conditional statements; if a condition doesn’t pass, then don’t do anything (effectively saying that the server rejects the client’s request and will not budge).
Remote.OnServerEvent:Connect(function (player, ...)
if conditionHere or conditionThere and conditionPossiblyHere then
whatToDoWhenOkay()
end
end)
7 Likes
In your case, you could add a debounce variable to your remote function, and reject any request that comes too quickly after the one before.
Using colbert2677’s implementation:
local debounce = -math.huge
Remote.OnServerEvent:Connect(function(player,...)
local now = os.clock()
if now - debounce <= 5 then return end
debounce = now
--do stuff
end)
4 Likes
The problem with that code is that it doesn’t account for other players.
It would require a lot of code I believe to make a system where it accounts for other players and has a way to determine if a remote event is actually be spammed.
For example, I might have a remote event that sends my mouse position every .5 seconds to the server and I might have a remote event I send every 10 seconds. You’ll need a way to set a definition for what counts as spamming for each remote event.
@TevinOfficial Why do you need to check though?
2 Likes
You could use a table of debounces for each player using the remote event/function. Of course, you would have to keep up with players joining and removing with this table, and then it would get messy, but it can be done:
local playerDebounces = {}
Players.PlayerAdded:Connect(function(player)
playerDebounces[player] = -math.huge
end)
Players.PlayerRemoving:Connect(function(player)
playerDebounces[player] = nil
end)
Remote.OnServerEvent:Connect(function(player, ...)
local now = os.clock()
if now - playerDebounces[player] <= 5 then return end
playerDebounces[player] = now
--do stuff
end)
You could also organize player-specific data in a folder instance in serverstorage, or things of that nature.
11 Likes
Imagine you have an FPS game in which you can fire a gun at only a set rate per second. Naturally, to make the guns FE compatible, you need to fire a remote event for when the user fires the gun. But, you have limited the fire rate on the client so you can give feedback.
However, an exploiter could spam the event over the amount and have an unfair advantage over other players.
So, if you should always double check that checks done on the client we’re done and done correctly, especially on the other end of a remote event.
Exploiters can do whatever a localscript can, so that includes firing remote events.
It’s good to assume your game is under attack, rather than leave it alone. Sanity checks should be pretty much everywhere.
4 Likes
You can take a look at this it has a similar question and idea as the OP
1 Like
I already do Sanity Checks on all my Remotes, op wants a Sanity check to figure out if his remotes are being spammed extremely fast, there’s a difference.
You can always use a variable and set it to true and have a check to see if it’s true set the variable to false wait until the remote function or event is done and once the remote event or function is done set it back to true
I was merely clarifying why that type of check is needed, as the user above asked why this kind of check would be needed.
I would disagree. I would consider this a kind of sanity check, as it checks the reliability of client behaviour / information on the server. This seems to me to be (pretty much) the definition of a sanity check.
However, if you want a solution, you could try having a dictionary where each key is a player’s userID or name (it doesn’t matter which). Then, store the denounce in there. To check for the denounce, simply use the player argument provided in a remote event handler and access the name.
EDIT: Sorry, I just realised that this is suggested already above:
An easy way is to use a Debounce.
It may be simple, but it helps to wait again to run the remote.
local Debounce = false
Remote.OnServerEvent:Connect(function(Player)
if not Debounce then
Debounce = true
--~ Function
Debounce = false
end
end)
But in cases of remote protection, you can check if the argument is wrong. If you have a wrong argument you can do a :Kick(), also because it is on the Server, it would be impossible for the exploit to bypass an argument error. But in some cases it may not be a good idea.
local Debounce = false
Remote.OnServerEvent:Connect(function(Player, Argument)
if not Debounce then
Debounce = true
if Argument == 'Reload' then
--~ Function
else
Player:Kick()
end
Debounce = false
end
end)
3 Likes
In almost all cases, kicking someone for failing a sanity check is a terrible choice. False positives exist and players should not be kicked for the faults of developers. Simply rejecting the request is sufficient enough (having the server not do anything if the client fails to pass the checks).
1 Like
In his case kicking the player has no consequences, since the remote has only one function, and he wrote the game code to ONLY send the 1 correct argument. Under these circumstances, there can be no false positives and any incorrect arguments can be assumed malicious.
But yes you are correct in saying that kicking usually isn’t the right choice
You can always add debounce. Here’s an example.
local CanShoot = true
local Cooldown = 0.3
ShootRemote.OnServerEvent(connect(function(arguments)
if CanShoot == true then -- if client tries to fire during the cooldown period (while the CanShoot value is false), the request is ignored
CanShoot = false -- disable ability to shoot
-- fire your gun
wait(Cooldown)
CanShoot = true
end
end)
This is an effective way to implement Cooldown, however it can suffer from server lag. One way to fix this is to implement the cooldown on both the client and server, the client having a slightly longer cooldown to account for server lag.
Kicking the player always has consequences. The code provided was merely an example, production variants of remotes can get larger. There’s absolutely no reason why the player should be kicked for failing a sanity check. Like I said, it can produce false positives or a developer fault can kick innocent players. Even then, there’s no reason to kick the player. Just reject the request.
3 Likes
This code wouldn’t work, as it assumes there is only one player. In this code, everybody will get the effect of the denounce one player has initiated, because there is only one debounce variable. This is why suggestions such as this were suggested above:
@ndrwcswll
You seem to be suggesting the same debounce mechanism which wouldn’t work.
Also, if there is only one value a remote event should hold, what is the point of having the argument? It just over complicates things. An argument is for use when you need to provide information. In this case, you aren’t doing this at all.
As @colbert2677 said previously, band-aid fixes are bad and will nearly never work. In this case, all the exploiter needs to do is find a single value and they can bypass this. Also, for this to work, the value has to appear somewhere in a local-script, which an exploiter can easily view using a Lua decompiler. This would make this long, fruitless effort go to waste, because they can then share the value. In short, this solution isn’t very good and can be bypassed very easily.
Finally, I’m not really sure why you need this at all. The debounce checks are done on the server, so why do you need to check if an exploiter is firing the remote event, or a local-script? They would both, essentially, have the same behaviour.
1 Like
The code you provided would not work. It would debounce all players at the same time. I’m not sure what you mean by “some remote that are only run in certain cases”, as a remote event will be being used by all players at the same time. There will be, pretty much, no situation in which a remote is only being used by a single client.
However, the code you provided as an alternative would work, but with certain drawbacks. Take a look at @goldenstein64’s solution above.
One thing to keep in mind with this debounce system is that if the function ever errors, your debounce will be stuck there forever.
You’re probably going to want to separate all your events so they each have their own debounce that expires after a certain amount of time(or perhaps it can handle back-to-back fires until it starts blocking you)
Building on @goldenstein64 answer, have you considered using a count when implementing sanity checks?
This enables a remove event/function to be called multiple times in a short period of time (e.g. if firing bullets from an automatic weapon) without triggering as many false positives or a lengthy cooldown.
For example:
--Variables
local requestsLimit = 20
local sanityChecks = {}
--Refresh Sanity Checks
coroutine.wrap(function()
while wait(5) do
sanityChecks = {}
end
end)()
--Main
Remote.OnServerEvent:Connect(function(player,...)
--Sanity Checks
local remoteName = Remote.Name
if not sanityChecks[player] then
sanityChecks[player] = {}
end
local requests = sanityChecks[player][remoteName]
if not requests then
requests = 1
sanityChecks[player][remoteName] = requests
else
sanityChecks[player][remoteName] = requests + 1
end
if requests > requestsLimit then
--Warn user is making too many requests
wait(1)
end
--Do your stuff
end)
4 Likes
This code is very well written and seems to get the job done really well. I like the idea of having a coroutine to handle refreshing of the debounces.
However, I would question if it’s a good idea to store the player object themself in the sanity array. Would it not be better to store player.UserId instead?