Just like the title suggests, I’ve created a voting system where a player is able to click on the physical appearance of another player with the cursor of their mouse. Every time a player is voted upon, a BillboardGui become visible above their head and shows the number of votes that player currently has.
Within my RemoteEvent, I have a ServerScript inside of it as well.
local Players = game:GetService("Players")
local RemoteEvent = script.Parent
RemoteEvent.OnServerEvent:Connect(function(player,target,Statusfolder,...)
if player.StatusFolder.hasVoted.Value == false then
--Loop through the players
for _, v in ipairs(Players:GetChildren()) do
--If target's name matches the Player's name.
if target.Name == v.Name then
if v.Name ~= player.Name then --This line just ensures that the player isn't voting themselves.
if v ~= nil and v.Character.Humanoid ~= nil then
--Adjusts the appropriate values.
v.StatusFolder.Votes.Value = v.StatusFolder.Votes.Value + 1
player.StatusFolder.hasVoted.Value = true
v.Character.Torso.Percentage_UI.Main.Vote_Count.TextLabel.Text = v.StatusFolder.Votes.Value
end
end
end
end
end
end)
This RemoteEvent is triggered by a ModuleScript from ReplicatedStorage, and that ModuleScript is triggered from the MouseManager LocalScript within the StarterPlayerScripts.
Chances are you might not understand what I just said, so here’s a more simplified rundown:
So how secured is this structure? Is there a backdoor that an exploiter can find through this? Specifically, is it a bad idea to but a RemoteEvent/Script inside the StarterGui? I’m not sure what other place I can put it in.
There’s not much leeway when you’re working with UserInputService and playing with SelectionBox instances, so I thought I’d ask.
You seem to pass Statusfolder and target as instances for the remote. I recommend not doing that. Instances can be spoofed with look-alike tables and that has its own issues.
The code itself doesn’t look vulnerable, though, but there are some changes I’d make.
blah_blah:Connect(function(player, targetName)
if player.StatusFolder.hasVoted.Value then
return
end
-- we avoid looping to find by just using `FindFirstChild`
local target = Players:FindFirstChild(targetName)
-- check that both the target is valid and not themselves
if not target or target == player then
return
end
-- check that the target has a humanoid
-- in your check you do `v ~= nil`, which wasn't required in the loop because `v` is always something
-- you also do `v.Character.Humanoid ~= nil`, which would error if a humanoid wasn't present
if not target.Character or not target.Character:FindFirstChild("Humanoid") then
return
end
-- first set the flag, then set the other values
player.StatusFolder.hasVoted.Value = true
-- Luau supports compound operators such as `+=` or `-=`
target.StatusFolder.Votes.Value += 1
target.Character.Torso.Percentage_UI.Main.Vote_Count.TextLabel.Text = target.StatusFolder.Votes.Value
end)
Now you may do this differently; it’s possible to merge some of the guard clauses or or invert them into a normal if statement if you want, but that’s up to your own preference. I also recommend creating a standard for naming for yourself. There is a bit of inconsistency in your instances specifically, with hasValue, Percentage_UI and Vote_Count.
Sorry I wasn’t very clear. player (the first argument) can’t be spoofed. player.StatusFolder is fine if player is the first argument that Roblox passes automatically to the remote.
You passed another argument, Statusfolder, which could be anything. It is possible to check it by:
typeof(Statusfolder) == 'Instance' and Statusfolder.ClassName == 'Folder' and Statusfolder.Parent == expected_parent
But that’s more effort than it’s worth. Since you already know the folder is located in the player instance, you can just normally index it without giving the client that control.
