"Exploitable" Scripts

Hello Developers!

I’m being told that my scripts are “exploitable”. Can anyone help? And explain more about things being exploitable.

local teamchange = game.ReplicatedStorage.Events.TeamChange

local function onteamchangeFired(plr, team)
    if plr.Team ~= game.Teams[team] then
       plr.Team = game.Teams[team]
       plr:LoadCharacter()
    end
end

teamchange.OnServerEvent:Connect(onteamchangeFired)
3 Likes

Basically, since you don’t verify if the player should be allowed in another team nor do you have a debounce, an exploiter can fire your remote to change teams whenever they please, even if they shouldn’t be able to so (i.e in the middle of a round).

1 Like

What do you mean by this? vvvv

Are you saying I should add an if statement verifying they are in the group required?

It really depends on your game and the types of teams.
If it let’s player choose a team before the round starts, you should make the remote not work when the round is in progress.
If you have teams which require a specific rank in your group, you should restrict the access to them from the remote aswell.

Basically assume that any player can fire the remote at any time with any team name, then think of the possible problems it could cause, and then try to fix it.

I’m a bit confused on how to go about this, would I check if the player is in the specified group? I have a folder of groups inside ReplicatedStorage, with IntValues of teams btw, and these have group ID’s as their values, but how would I go about defining them?

You can use Player:IsInGroup(group id) to check if a player is in a group.
Not sure what do you mean by “IntValues of teams”. IntValues named the team’s name with groups ids as their values? If that’s the case, you could do it like this:

local function OnTeamChangeFired(plr, team)
    local intVal = game.ReplicatedStorage:FindFirstChild(team)

    local team = game:GetService("Teams"):FindFirstChild(team)

    if plr.Team ~= team and plr:IsInGroup(intVal.Value) then
        plr.Team = team
        plr:LoadCharacter()
    end
end

You might want to implement some cache so you dont have to call Player:IsInGroup() each time.

7 Likes

To add a little bit to what @Amiaa16 said, you should always add server sided checks to check anything the client sends to the server. The general rule is to never trust the client meaning that you should always assume the information sent from the client has been tampered with. This is why developers try to keep most of their important game logic as server sided as possible to reduce the chance of it being exploited.

Answer to your question about things being exploitable:
Anything is exploitable on the client as the client has access to it but it is what the exploiter can do with what they have access to. Lets say we have a basic gui shop with different items and whenever the client clicks the item they buy it and it gives them the item. This is a huge issue because the client could just exploit the shop and get any item they want. To make this shop more secure you would need to involve the server to do some server sided checks. When making any sort of system like this the server should be the one checking if the purchase can happen not the client. Having these server sided checks makes things less exploitable.

In your code, you have provided a way for an exploiter to change their team as they please because you are relying on the client to tell you what team they want to be changed to. This can be easily fixed by having a server sided check that checks if the player is actually allowed to change their team.

Notes:

  • Never have client sided checks as the client can tamper with them.

  • Having the server involeved is pointless unless you have server sided checks.

  • Remember never trust the client.

You may want to read over this topic about exploiting to get a better understanding of it: Exploiting Explained

4 Likes