How to write a safe UpdateAsync callback

What it looks like

The function passed to UpdateAsync does more than work out the new value. It gives the player coins or items, changes other tables, fires remotes, or yields.

store:UpdateAsync(key, function(old)
	profile.Coins += old.reward
	return nil
end)

How to detect it

• –Find every UpdateAsync call and read its callback.
• –The callback should only read the old value and return the new one, or nil to cancel. Anything else it changes outside itself, such as a profile, a player, a remote or another table, is a finding. Setting a local variable that the code reads after the call returns is fine.
• –The callback runs before the write is confirmed. If the write then fails, is throttled and dropped, or errors on a value that's too large, the change outside it has already happened while the data store still holds the old value.
• –Returning nil cancels the write, so any grant made before that return nil stays without anything being saved.
• –Roblox's documentation says the callback cannot yield. Calls such as task.wait, GetAsync or InvokeClient inside it are a finding too.

What an exploiter could do

The usual case is a store of things owed to the player, such as compensation, mail or trade proceeds. The callback pays the player and returns an empty list. When the write fails, the player keeps the payment and the list stays as it was, so the next collection pays again. Anyone who can make writes fail, for example by spamming an unthrottled remote that uses up the server's data store budget, can repeat this on purpose.

How to fix it

• –Keep the callback pure. Read the old value, copy what you need into a local variable, and return the new value.
• –Grant only after UpdateAsync returns successfully, using what the callback copied out.
• –Give each entry an id and record granted ids in the player's saved profile. Before granting, skip ids already recorded, and let a later update remove entries the profile has already claimed. Then a failed or repeated update can't pay twice.
• –Do any yielding work before or after the call, never inside it.

Vulnerable example

local DataStoreService = game:GetService("DataStoreService")
local Players = game:GetService("Players")
local PlayerData = require(script.Parent.PlayerData)

local store = DataStoreService:GetDataStore("Mail")

Players.PlayerAdded:Connect(function(player)
	local profile = PlayerData.waitFor(player)
	if not profile then
		return
	end
	pcall(function()
		store:UpdateAsync(tostring(player.UserId), function(mail: unknown): { string }?
			if typeof(mail) ~= "table" then
				return nil
			end
			for _, item in mail :: { unknown } do
				if typeof(item) == "string" then
					table.insert(profile.Inventory, item)
				end
			end
			return {}
		end)
	end)
end)

Fixed example

local DataStoreService = game:GetService("DataStoreService")
local Players = game:GetService("Players")
local PlayerData = require(script.Parent.PlayerData)

type Letter = {
	id: string,
	item: string,
}

local store = DataStoreService:GetDataStore("Mail")

local function readMail(mail: unknown): { Letter }
	local letters: { Letter } = {}
	if typeof(mail) ~= "table" then
		return letters
	end
	for _, entry in mail :: { unknown } do
		if typeof(entry) == "table" then
			local fields = entry :: { [string]: unknown }
			local id, item = fields.id, fields.item
			if typeof(id) == "string" and typeof(item) == "string" then
				table.insert(letters, { id = id, item = item })
			end
		end
	end
	return letters
end

Players.PlayerAdded:Connect(function(player)
	local profile = PlayerData.waitFor(player)
	if not profile then
		return
	end
	local unread: { Letter } = {}
	local ok = pcall(function()
		store:UpdateAsync(tostring(player.UserId), function(mail: unknown): { Letter }?
			unread = {}
			if typeof(mail) ~= "table" then
				return nil
			end
			for _, letter in readMail(mail) do
				if not profile.Claimed["Mail:" .. letter.id] then
					table.insert(unread, letter)
				end
			end
			return unread
		end)
	end)
	if not ok or PlayerData.get(player) ~= profile then
		return
	end
	for _, letter in unread do
		local key = "Mail:" .. letter.id
		if not profile.Claimed[key] then
			profile.Claimed[key] = true
			table.insert(profile.Inventory, letter.item)
		end
	end
end)

Common false positives

• –Setting a local variable declared outside the callback, so the code after the call can use what was read, is the recommended pattern. Reset it at the start of the callback.
• –Reading other tables inside the callback, such as checking a profile's claimed ids, isn't a side effect. Only changes count.
• –A session-lock callback that sets a local "claimed" result and returns the locked record, like the fix in Player data with no session lock, is fine.