So a last night my game was hacked by a script kiddie and a good chunk of our datastores were messed with. While my staff team is working on manually fixing data, the malicious code still persists, so I’m trying to implement a sanity check into the script it’s effecting- my leaderstat manager.
I tried setting up a variable, timesave, to properly save the user’s correct time. Because only the leaderstat is affected, the timesave variable should be correct.
However, this isn’t working.
How can I fix this? Should I just make two datastores and make them check off of each other?
local DataStoreService = game:GetService("DataStoreService")
local BadgeService = game:GetService("BadgeService")
local Players = game:GetService("Players")
local myDataStore = DataStoreService:GetDataStore("TimeData")
Players.PlayerAdded:Connect(function(player)
local leaderstats = Instance.new("Folder")
leaderstats.Name = "leaderstats"
leaderstats.Parent = player
local Time = Instance.new("IntValue")
Time.Name = "Minutes"
Time.Parent = leaderstats
local timesave = 0
local playerUserId = "Player"..player.UserId
local data
local success, errormessage = pcall(function()
data = myDataStore:GetAsync(playerUserId)
end)
if success then
player:SetAttribute("DataLoaded", true)
Time.Value = data or 0 --loads data when player joins
timesave = Time.Value
end
while player do
player.leaderstats.Minutes.Value += 1 --makes time go up by one every minute
task.wait(60)
-- bunch of badge-related code here that isnt relevant
end
end
while player do
timesave += 1
if timesave ~= player.leaderstats.Minutes.Value then
player.leaderstats.Minutes.Value = timesave
end
wait(60)
print("tick")
end
end)
Players.PlayerRemoving:Connect(function(player)
if player:GetAttribute("DataLoaded") ~= true then -- If the players data has not been loaded we don't want to overwhite it
warn(string.find("%s data was not saved as it failed to load."), player.Name)
return
end
local playerUserId = "Player"..player.UserId
local data = player.leaderstats.Minutes.Value
if data <= -1 then
warn(string.find("%s data was tampered with and did not save."), player.Name)
return
end
local success, errormessage = pcall(function()
myDataStore:SetAsync(playerUserId, data) --saves data when player leaves game
end)
if not success then
warn(string.find("%s data failed to load."), player.Name)
end
end)
Here is the original code without the added attempt to save data to another variable:
local DataStoreService = game:GetService("DataStoreService")
local BadgeService = game:GetService("BadgeService")
local Players = game:GetService("Players")
local myDataStore = DataStoreService:GetDataStore("TimeData")
Players.PlayerAdded:Connect(function(player)
local leaderstats = Instance.new("Folder")
leaderstats.Name = "leaderstats"
leaderstats.Parent = player
local Time = Instance.new("IntValue")
Time.Name = "Minutes"
Time.Parent = leaderstats
local playerUserId = "Player"..player.UserId
local data
local success, errormessage = pcall(function()
data = myDataStore:GetAsync(playerUserId)
end)
if success then
player:SetAttribute("DataLoaded", true)
Time.Value = data or 0 --loads data when player joins
end
while player do
player.leaderstats.Minutes.Value += 1 --makes time go up by one every minute
task.wait(60)
-- bunch of badge-related code here that isnt relevant
end
end
end)
Players.PlayerRemoving:Connect(function(player)
if player:GetAttribute("DataLoaded") ~= true then -- If the players data has not been loaded we don't want to overwhite it
warn(string.find("%s data was not saved as it failed to load."), player.Name)
return
end
local playerUserId = "Player"..player.UserId
local data = player.leaderstats.Minutes.Value
if data <= -1 then
warn(string.find("%s data was tampered with and did not save."), player.Name)
return
end
local success, errormessage = pcall(function()
myDataStore:SetAsync(playerUserId, data) --saves data when player leaves game
end)
if not success then
warn(string.find("%s data failed to load."), player.Name)
end
end)
If you can, please provide script examples. Thanks!
The source of the problem is not here, I do notice you have 2 while loops 1 above the other which will not run but that’s not related to your question.
If you are certain your game does not have a backdoor and you think it’s a vulnerability to your remotes then the problem is the script handling your remotes, not your datastore script.
Sanity checks like you mentioned is meant to secure remote events and remote functions.
I went in and removed my unused remotes. I literally do not know how a remote would be affecting my leaderstats when there are no remotes that do anything to the leaderstats.
Not to mention I could just fix it in this script anyway, because it is only affecting the leaderstats. I could run a check to see if the data is wrong and if it is, fix it. I don’t see how my logic here is wrong.
An exploiter can use applications like remotespy to see and modify what is being sent or received to the server from remotes, The remote does not necessarily have to be related to your leaderstats.
--// VULNERABLE REMOTES
--SERVER
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RE = ReplicatedStorage.RemoteEvent
function SetTime(Player, Target, Value)
Target.Value = Value
end
RE.OnServerEvent:Connect(SetTime)
--CLIENT
local Player = game:GetService("Players").LocalPlayer
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RE = ReplicatedStorage.RemoteEvent
local Minutes = Player.leaderstats.Minutes
RE:FireServer(Minutes, -10000000)
--// SECURED REMOTES
--SERVER
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RE = ReplicatedStorage.RemoteEvent
local PlayerDebounce = {}
function AddTime(Player)
if table.find(PlayerDebounce, Player) then return end
table.insert(PlayerDebounce, Player)
local leaderstats = Player:FindFirstChild("leaderstats")
if leaderstats then
local Minutes = leaderstats:FindFirstChild("Minutes")
if Minutes then
Minutes.Value += 10
end
end
task.wait(3)
table.remove(PlayerDebounce, table.find(PlayerDebounce, Player))
end
RE.OnServerEvent:Connect(AddTime)
--CLIENT
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RE = ReplicatedStorage.RemoteEvent
while task.wait() do
RE:FireServer() -- With this example the server is checking the debounce on the server side so this will not add the time continuosly
end
This is an example of a remote vulnerability obviously not the best example but you get the point. A remote vulnerability does not always have to be related to that specific task you are trying to perform. Like I said in your other post I had once work on a game where an exploiter was able to change leaderstats value through an unsecured “handcuff” tool.
I think you’re going to need to explain this to me like I’m 5. I don’t know anything about RemoteEvents and avoid using them as much as possible due to my lack of knowledge on them. I don’t even know how to find a vulnerability like this.
And I don’t see why I still couldn’t just run a check in this code and fix the leaderstat there. This post isn’t about Remotes, it’s about fixing this from the end of my manager script.
There’s no point just resetting the values or checking if the values hit a certain threshold in your leaderstats script as the whole point is to prevent exploiters in the first place, otherwise exploiters can just change the values.
You have to fix the problem at its source which is the remotes. If you claim these exploiters are script kiddies you would know more about remotes than them.
This video will explain more on remote vulnerability better as my english is not that great.
The source of your issue probably isn’t in this script, since there’s nothing listening to the client in it.
Irrelevant rant about handling player state in a module that you can read if you want
I figured maybe since you were handling data directly, which is stored under the player instance, but I tested in Studio and changes from the client do not replicate to the server even for leaderstats values. I would still recommend not dealing with this data directly in the leaderstats values, but instead creating a module to provide an interface for dealing with this data and then making the leaderstats values merely reflect this data for a few reasons:
If you have any admin systems (i.e. adonis) installed, this makes it so that malicious admins (or even exploiters who have a backdoor) can’t directly set their minutes via stats related commands.
Even if you don’t have any admin systems, a thoughtfully designed interface will make it difficult to create this kind of vulnerability (and also block exploiters with backdoors from mutating their minutes state; this might solve your problem altogether!)
Now, to actually diagnose your issue, do you have any remote events at all that, when executed, have any code path that might wind up changing the players’ minutes?
technically you could but its a bit of a bandaid fix. Bandaid fixes are still fixes though so I’m not judging. If you want to go this route, you’re on the right track — just move the time check logic into the same while loop as the incrementing stuff, like so (NOTE: UNTESTED!!! !!! !!! might not work.):
while player do
if timesave ~= player.leaderstats.Minutes.Value then
player.leaderstats.Minutes.Value = timesave
end
player.leaderstats.Minutes.Value += 1 --makes time go up by one every minute
timesave += 1
task.wait(60)
-- bunch of badge-related code here that isnt relevant
end
end
The reason for this is because in the code you posted, the first loop runs until the player is gone, in which case the second loop that handles the timesave stuff will never run.
None. That’s what I’ve been trying to say to the guy above. All minute changes are handled by this script and this script alone. I have stated this is almost definitely a script injection, as it is running a suspicious code that I have identified, I just don’t know how to get it out. Exploiter thought they were funny putting a print function in, but it just ratted them out:
I’ll try your fix later and get back to you then, thanks.
You would have to invite a developer into your project to get a more experienced look at your codebase. Preferably an ex-exploit developer. I recommend going HiddenDevs or Roblox Studio Community on Discord for Screen-Share guided advice as well
You have a backdoor in your game that’s not a remote problem, even so my point still stand why are you trying to mitigate a problem by doing checks, you should remove the code that contains this malicious code.
I understand what you mean, but try to understand that the values can be modified by any script and not just the one you showed us, the ones you showed do not seem to have any error or any kind of vulnerability for exploiters, so probably there is another place that is modifying your “Minutes” values and it is not exactly this script that you showed us.
Absence of a direct modification of the “Minutes” value is also not confirmation that no vulnerability exists. Exploiters are people who exploit vulnerable code; you could have code that indirectly modifies values that the exploiter can influence to target the “Minutes” value. Be careful of accepting user input from the client
a tip to find possible errors, check where you change the value, there is a tool where you can see all scripts that modify “Minutes”, it is not certain that you will find it because you may end up changing the variable. but try using this:
In any case, it’s relevant to point out that the “tampered” “Minutes” value is showing signs of integer overflow rather than a leveraged number. Integer overflow occurs when the maximum permissible number representable by the datatype IntValue uses (64-bit signed) is exceeded. The binary representation of the number via two’s complement causes the number to wrap around to the smallest representable value (-9.2 quintillion)
I ruled out the idea of it being a backdoor because you said they were script kiddies and you were going to do sanity checks and nobody do sanity checks on backdoors.
I’d assume you know what you were talking about, my apologies. I did say if you were certain it’s not a backdoor then it should be a remote problem.
You can try finding the problem by searching with ctrl + shift + f and try to find words like:
If you read my first post, you would know that the exploiters were setting peoples’ minutes to be in the negative 9 somethings.
Besides, minutes is basically never going to overflow. It only increments 1 per minute. Unless you want to play my game for 17,000,000,000,000 years, then sure!
Did you manage to find the error? If not, I don’t know if you can trust me to put me in your game, but that’s okay if not. If you can send me a copy of the map so I can try to help.
