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?
The only caveat with not using the player’s UserId is that if the sanity check table doesn’t refresh, then a reference to the player will be kept which will prevent their object from being garbage collected should they disconnect from the game. Otherwise, there’s no real difference between using it or not.
1 Like
The reason people kick the player is the same reason they ban them; to deter any future exploiting attempts and to remove them from the game. If your game is well thought out and well executed there shouldn’t be any incorrect parameters sent from the localscripts.
Manual moderation is different from embedding a kick function into your scripts for a failed sanity check, whether that’s a developer-made fault or one that’s generated from spoofed or wrong arguments. Manual moderation is a band-aid solution to “deter future exploiting attempts” which is only relevant to a single account. It’s not scalable or effective. The proper solution is to fix your game.
1 Like
I think it’s just as valid as any other vote to kick system since they are both checking for exploiters.
Regardless, I’m by no means saying that you should always kick people who failed sanity checks (do to false positives) however there are some situations where you can be 100% sure that a player is exploiting, and it requires immediate action. An example of this would be when phantom forces kicks you for fly hacking, or when jailbreak kicks you for no clip: both of these are situations where there is no room for false positives (the game is sure the player is hacking) and kicking the player is actually required. Anyways, I understand now that spoofed or incorrect arguments sent through remotes do not require immediate action or attention, and so ignoring the request instead of punishing the player is the correct choice in that scenario.
Vote kick is also a form of manual moderation, by community instead of by staff.
Kicking players for explicitly exploiting is fair in itself, if that’s the intended function of your script. I’m generalising my response towards sanity checks of remotes in general though.
If you’re not explicitly checking for those kinds of exploits with remotes or whatever and it’s simply a remote for client-server communication for some kind of game functionality, there’s no reason to kick a player - just reject the request or do nothing if the checks fail. If a player does get kicked for hitting a false positive or something that’s the fault of the developer, that’s dangerous for UX and it doesn’t really help anyone either. A simple request reject allows everyone to continue playing, even in error (not as in a script-terminating error though).
Anyway, that being said, feel free to hit my DMs if you’re interested in further discussion or anything. I think we’ve dragged this topic out a bit, lol.
1 Like
Hey, I know it’s been a year but it’s still a very important topic,
So how I do it is-
-- Server
local RemoteEvent = game.ReplicatedStorage["NAME HERE"]
RemoteEvent.OnServerEvent:Connect(function(client, argument1)
if argument1 == 'ye this is it' then
-- code here
end
end)
-- Client
-- code here
game.ReplicatedStorage['name here']:FireServer('ye this is it')
1 Like
You have the right idea, but what you’ve currently ran into is a password-protected remote which is not real security. It could be a different story if your script actually needed to send a string though and the server needs to either validate or sanitise it.
If you’re validating a string for example and don’t want it to exceed a certain number of characters, you can check the length of the string using string.len or the length operator (#). Then after getting the number, check if its under a certain amount. If it is, then proceed with your code. If not, then end it there.
If you’re sanitising a string for example and don’t want it to exceed a certain number of characters, you would use string.sub in order to cut off excess, so that the server is only working with the first 200 characters of the string or so. You can then proceed using that sanitised string.
A personal favourite thing of mine with regards to sanity checks is guard clauses, so I can “detach” my checks from my code. So rather than an if statement encapsulating all my code, its just a line or some that act on the received arguments.
OnServerEvent:Connect(function (player, a)
if #a > 200 then return end
-- Code here
end)
3 Likes