Improve code to prevent hacking

Right now I have this LocalScript that when a Gui button is pressed, depending on the order multiple of these buttons are pressed in will result in different animations playing only when 2 buttons are pressed. And when any of the buttons are pressed, it will result in one of the players currency (being tubes in this case) being deducted. If the player doesn’t have any currency left then the action simply won’t be carried out.

Problem is this is in a LocalScript. And I know yes ofc this is easily exploitable as it’s in a LocalScript. I’m just not too sure how to go about converting parts of this to a ServerScript as the animations that need to be carried out kindof have to be local so that they look smooth to the player and if any check is put in the LocalScript then wouldn’t a hacker just simply be able to delete the code?

tubes[1].Activated:Connect(function() --An table of the Gui buttons. This one being the first one
--a hacker could just go into the LocalScript and simply delete this check which is what needs to be prevented
	if potionAmount.Value > 0 then --the players currency that is a intValue inside the LocalPlayer
		vChange:FireServer(script.Name) --remoteEvent that reduces one of the players currency on the server side (currency being tubes)
	else 
		print("no more tubes available")
		return
	end

--everything from here on is mostly just toggling Guis and playing sounds
	local tubeRandom = math.random(1, #tubeAudio) --just a table of possible noises that randomly picks one

	if selectionOne.Value == "" then --if the first Gui button has not been selected
		selectionOne.Value = tubes[1].Name
		tubes[1].Active = false
		tubes[1].ImageTransparency = 0.5	

		oneTubeButtonPressedTrue:FireServer() --precaution for if the player backs out while only using one tube then it gives it back (2 are needed to play the animation)
		moduleScript.oneTubeButtonPressed = true
		tubeAudio[tubeRandom]:Play()		
	elseif selectionOne.Value ~= "" then
		-- Makes the rest of the options unusable
		oneTubeButtonPressedFalse:FireServer() 
		moduleScript.oneTubeButtonPressed = false
		task.wait()

		twoTubeButtonPressedTrue:FireServer() 
		moduleScript.twoTubeButtonPressed = true

		tubes[1].Active = false
		tubes[2].Active = false
		tubes[3].Active = false
		tubes[4].Active = false
		tubes[5].Active = false

		if pouring.IsPlaying then
			pouring.Ended:Wait()
		end	
		if pouringMix.IsPlaying then
			pouringMix.Ended:Wait()
		end	
		selectionTwo.Value = tubes[1].Name

		tubes[1].ImageTransparency = 0.5
		tubes[2].ImageTransparency = 0.5
		tubes[3].ImageTransparency = 0.5
		tubes[4].ImageTransparency = 0.5
		tubes[5].ImageTransparency = 0.5

		tubeAudio[tubeRandom]:Play()
	end
end)
1 Like
vChange:FireServer(script.Name, potionAmount.Value) --remoteEvent that reduces one of the players currency on the server side (currency being tubes)

You can use a sanity check, sanity checks are basically just a way of verifying information passed from the client to the server. In your case, your server-check would be like this:

vChange.OnServerEvent:Connect(function(Player, script.Name, potionAmount) -- I added in potion amount as a third value that you should send from the client when firing the remote
if potionAmount > 0 then
-- Run your code here
end)

Also get rid of the if statement on the client and instead put it on this server side check, so that the client can’t get rid of it.

If potionAmount is a value saved in a key of a datastore, then you can ensure that your code is 100% protected by using :GetAsync() from the datastore and inserting your key which returns the value that was saved, while this ensures 100% safety as hackers won’t be able to tamper with the actual data. Calling getasync is not the best idea, especially since if you were firing your remote event everytime the gui button is activated. A player could just randomly keep clicking the button and then the server-side would error for calling too much getasync.

Wouldn’t the client still be able to delete the :FireServer()? My only idea to counter this would be to put the entire LocalScripts function into the .OnServerEvent:Connect() but I’m thinking that could cause issues with the timing and playing of animations.

Handle everything important on the server, use validation checks server side to ensure that the requested action is actually allowed. Use the client to handle animations, non-critical things that need to happen instantly (eg GUI stuff), and things that only occur for that specific player (eg playing sounds).

Use :FireServer(…) to request that the server tries to do something
Use .onServerEvent:Connect(function(plr,…) end) to respond to requests from the client.

When the event signal is recieved by the server, it always passes the Player object for the client the signal is from. When handling currency, always check if the determining factors are valid on the server using that passed plr parameter. EG, check that the player has enough money to make a purchase server side. Make sure the player has actually completed a task before adding money via the server side. Always assume the hackers can call :FireServer(…) with any data.

But if they did that, they’re ruining themselves. Because then nothing will happen. So they can’t get any benefits either.