Players can steal money using donation system

Help and Feedback Scripting Support
#1

In my game I have a donation system that allows players to donate to each other. Apparently if you put in a - number you can actually steal money from the other player. How do I fix this?

Code

``

SendMoney.SendButton.MouseButton1Click:Connect(function()
	Sounds.ButtonClick:Play()
	local amount = tonumber(SendMoney.AmountBox.Text)
	if amount then
		local otherPlayer = PlayersService:FindFirstChild(SendMoney.PlayerBox.Text)
		if otherPlayer then
			if otherPlayer == Player then print('You cant send money to yourself') return end
			if PlayerData.Drachma >= amount and amount <= 5000 then
				Events.SendMoney:FireServer(amount,otherPlayer)
			elseif amount > 5000 then
				print("Sorry, you can only donate up to 5000!")
			else
				print('Sorry, you do not have enough money!')
			end
		else
			print('Sorry"'..SendMoney.PlayerBox.Text..'" Is not a valid player!')
		end
	else
		print('Sorry, You must enter in a valid number to send! "'.. SendMoney.AmountBox.Text .."' Is not a number!")
	end
end)
#2

Check to see if the amount > 0 - If it is, then follow your code.

SendMoney.SendButton.MouseButton1Click:Connect(function()
	Sounds.ButtonClick:Play()
	local amount = tonumber(SendMoney.AmountBox.Text)
	if amount and amount > 0 then
		local otherPlayer = PlayersService:FindFirstChild(SendMoney.PlayerBox.Text)
		if otherPlayer then
			if otherPlayer == Player then print('You cant send money to yourself') return end
			if PlayerData.Drachma >= amount and amount <= 5000 then
				Events.SendMoney:FireServer(amount,otherPlayer)
			elseif amount > 5000 then
				print("Sorry, you can only donate up to 5000!")
			else
				print('Sorry, you do not have enough money!')
			end
		else
			print('Sorry"'..SendMoney.PlayerBox.Text..'" Is not a valid player!')
		end
	else
		print('Sorry, You must enter in a valid number to send! "'.. SendMoney.AmountBox.Text .."' Is not a number!")
	end
end)
2 Likes
#3

Make sure the money is greater than zero on the server as well

3 Likes
#4

It already should be, as you send the amount inside the if statement. If it’s not already above 0, it wont reach the :FireServer(amount, otherPlayer) event.

It is a good safety measure though.

3 Likes
#5

Doesn’t matter, exploiters can fire any remotes with what they want so it’s 100% necessary to prevent players from stealing money through this.

3 Likes
#6

I did not think of this, thank you. Yes, 100% check it on the server as well OP.

2 Likes