Script security: Difference between revisions

From Multi Theft Auto: Wiki
Jump to navigation Jump to search
Line 43: Line 43:
* While reviewing scripts for possible security holes. Look at any data coming from the client that is being trusted.
* While reviewing scripts for possible security holes. Look at any data coming from the client that is being trusted.
* Any kind of data could be sent, hence server scripts which communicate with client by receiving data sent by players should validate it, before further use in latter parts of code. Mostly, it will be done either by [[setElementData]] or [[triggerServerEvent]].
* Any kind of data could be sent, hence server scripts which communicate with client by receiving data sent by players should validate it, before further use in latter parts of code. Mostly, it will be done either by [[setElementData]] or [[triggerServerEvent]].
* You shouldn't rely only on player [https://wiki.multitheftauto.com/wiki/Serial serial], when it comes to processing crucial operations. '''Serials aren't guaranted to be unique or non-fakable'''. This is why you should '''combine it with''' account system (e.g: '''login & password''').
* You shouldn't rely only on player [https://wiki.multitheftauto.com/wiki/Serial serial], when it comes to processing crucial operations (auto-login/admin actions). '''Serials aren't guaranted to be unique or non-fakable'''. This is why you should '''put it behind''' account system, as '''important authentication factor''' (e.g: '''login & password''').
* Server-side logic '''can not be bypassed''' or '''tampered''' with (unless server is breached or when there is a bug in code, but that's whole different scenario) - '''use it to your advantage'''. In majority of cases, you will be able to perform security validations with no participation of client-side.
* Server-side logic '''can not be bypassed''' or '''tampered''' with (unless server is breached or when there is a bug in code, but that's whole different scenario) - '''use it to your advantage'''. In majority of cases, you will be able to perform security validations with no participation of client-side.
* Using concept of '''''“All parameters including source can be faked and should not be trusted. Global variable client can be trusted.”''''' - gives you reliable assurance that player to which you refer (via '''client''') '''is''' in fact, '''the one which really called event'''. This approach will protect you from situations where cheater can call (and process) admin events (e.g kick/ban player) by passing the actual admin ('''2nd''' argument in '''triggerServerEvent''') - as a consequence of using wrong variable. To make sure you fully understood it, take a look at examples below:
* Using concept of '''''“All parameters including source can be faked and should not be trusted. Global variable client can be trusted.”''''' - gives you reliable assurance that player to which you refer (via '''client''') '''is''' in fact, '''the one which really called event'''. This approach will protect you from situations where cheater can call (and process) admin events (e.g kick/ban player) by passing the actual admin ('''2nd''' argument in '''triggerServerEvent''') - as a consequence of using wrong variable. To make sure you fully understood it, take a look at examples below:

Revision as of 15:44, 24 June 2023

Awareness of client memory

Starting from very basics:

  • You should be aware that everything you store on client-side is at risk, this includes Lua itself. Any confidential (and/or) important data could be accessed by malicious clients.
  • To keep sensitive data (and/or) Lua logic safe - use server-side.
  • Do note that scripts marked as shared act also as client code, which means that everything above applies to them. For example defining:
<script src="script.lua" type="shared"/> <!-- this script will run separately both on client and server -->

Is same as doing:

<script src="script.lua" type="client"/> <!-- define it separately on client -->
<script src="script.lua" type="server"/> <!-- do the same, but on server -->

Additional protection layer

In order to make things slightly harder* for those having bad intentions towards your server, you can make use of cache attribute (and/or Lua compile (also known as Luac) with extra obfuscation set to level 3 - API) available in meta.xml, along with configuring MTA's built-in AC by toggling SD (Special detections), see: Anti-cheat guide.

<script src="shared.lua" type="shared" cache="false"/> <!-- cache="false" indicates that this Lua file won't be saved on player's PC -->
<script src="client.lua" type="client" cache="false"/> <!-- cache="false" indicates that this Lua file won't be saved on player's PC -->
  • Slightly harder but not impossible for some to obtain client code, yet does good job at keeping most people away from inspecting your .lua files - those looking for possible logic flaws (bugs) or missing/incorrect security based checks.
  • Can be used on both client and shared script type (has no effect on server-sided Lua).
  • It doesn't remove Lua files which were previously downloaded.

Detecting and dealing with backdoors and cheats

To ensure minimum (or no) damage resulting from Lua scripts:

  • Make sure to properly configure ACL (Access Control List) - which will block resources from using certain, potentially dangerous functions.
  • Zero-trust with giving away admin rights for resources (including maps) coming from unknown sources.
  • Before running any resource you don't trust, analyze:
    • meta.xml of it, for possible hidden scripts lurking beneath other file extensions.
    • It's source code, for malicious logic.
  • Do not run/keep using compiled resources (scripts) of which legitimacy you aren't sure.

To ensure minimum damage when a cheater connects to your server:

  • When making scripts, remember to never trust data coming from a client.
  • While reviewing scripts for possible security holes. Look at any data coming from the client that is being trusted.
  • Any kind of data could be sent, hence server scripts which communicate with client by receiving data sent by players should validate it, before further use in latter parts of code. Mostly, it will be done either by setElementData or triggerServerEvent.
  • You shouldn't rely only on player serial, when it comes to processing crucial operations (auto-login/admin actions). Serials aren't guaranted to be unique or non-fakable. This is why you should put it behind account system, as important authentication factor (e.g: login & password).
  • Server-side logic can not be bypassed or tampered with (unless server is breached or when there is a bug in code, but that's whole different scenario) - use it to your advantage. In majority of cases, you will be able to perform security validations with no participation of client-side.
  • Using concept of “All parameters including source can be faked and should not be trusted. Global variable client can be trusted.” - gives you reliable assurance that player to which you refer (via client) is in fact, the one which really called event. This approach will protect you from situations where cheater can call (and process) admin events (e.g kick/ban player) by passing the actual admin (2nd argument in triggerServerEvent) - as a consequence of using wrong variable. To make sure you fully understood it, take a look at examples below:
--[[
	DON'T EVER DO THAT - THAT IS COMPLETELY WRONG AND INSECURE
	THE ISSUE: BY USING 'source' IN hasObjectPermissionTo YOU ARE LEAVING DOOR STRAIGHT OPEN FOR CHEATERS
]]

function onServerWrongAdminEvent(playerToBan)
	if not client then -- this shouldn't happen, but for sanity let's check it
		return false -- it's not present, don't go further
	end

	local defaultPermission = false -- do not allow action by default, see (defaultPermission): https://wiki.multitheftauto.com/wiki/HasObjectPermissionTo
	local canAdminBanPlayer = hasObjectPermissionTo(source, "function.banPlayer", defaultPermission) -- the vulnerability lies here...

	if not canAdminBanPlayer then -- if player doesn't have permissions
		return false -- don't do it
	end

	local validElement = isElement(playerToBan) -- check whether argument passed from client is an element

	if not validElement then -- it is not
		return false -- stop code processing
	end

	local elementType = getElementType(playerToBan) -- it's an element, so get it's type
	local playerType = (elementType == "player") -- make sure that it's a player

	if not playerType then -- it's not a player
		return false -- stop here
	end

	-- banPlayer(...) -- do what needs to be done
end
addEvent("onServerWrongAdminEvent", true)
addEventHandler("onServerWrongAdminEvent", root, onServerWrongAdminEvent)

--[[
	onServerCorrectAdminEvent IS PERFECTLY SECURED, AS IT SHOULD BE
	NO ISSUE HERE: WE'VE USED 'client' IN hasObjectPermissionTo WHICH MAKES IT SAFE FROM FAKING PLAYER WHO CALLED EVENT
]]

function onServerCorrectAdminEvent(playerToBan)
	if not client then -- this shouldn't happen, but for sanity let's check it
		return false -- it's not present, don't go further
	end

	local defaultPermission = false -- do not allow action by default, see (defaultPermission): https://wiki.multitheftauto.com/wiki/HasObjectPermissionTo
	local canAdminBanPlayer = hasObjectPermissionTo(client, "function.banPlayer", defaultPermission) -- is player allowed to do that?

	if not canAdminBanPlayer then -- if player doesn't have permissions
		return false -- don't do it
	end

	local validElement = isElement(playerToBan) -- check whether argument passed from client is an element

	if not validElement then -- it is not
		return false -- stop code processing
	end

	local elementType = getElementType(playerToBan) -- it's an element, so get it's type
	local playerType = (elementType == "player") -- make sure that it's a player

	if not playerType then -- it's not a player
		return false -- stop here
	end

	-- banPlayer(...) -- do what needs to be done
end
addEvent("onServerCorrectAdminEvent", true)
addEventHandler("onServerCorrectAdminEvent", root, onServerCorrectAdminEvent)

Securing setElementData

  • All parameters including source can be faked and should not be trusted.
  • Global variable client can be trusted.

Click to collapse [-]
Server

Example of basic element data anti-cheat.

local function reportAndRevertDataChange(clientElement, sourceElement, dataKey, oldValue, newValue) -- helper function to log and revert changes
	local logClient = inspect(clientElement) -- in-depth view of player which forced element data sync
	local logSerial = getPlayerSerial(clientElement) or "N/A" -- client serial, or "N/A" if not possible, for some reason
	local logSource = tostring(sourceElement) -- element which received data
	local logOldValue = tostring(oldValue) -- old value
	local logNewValue = tostring(newValue) -- new value
	local logText = -- fill our report with data
		"=======================================\n"..
		"Detected element data abnormality:\n"..
		"Client: "..logClient.."\n"..
		"Client serial: "..logSerial.."\n"..
		"Source: "..logSource.."\n"..
		"Data key: "..dataKey.."\n"..
		"Old data value: "..logOldValue.."\n"..
		"New data value: "..logNewValue.."\n"..
		"======================================="

	local logVisibleTo = root -- specify who will see this log in console, in this case each player connected to server

	outputConsole(logText, logVisibleTo) -- print it to console
	setElementData(sourceElement, dataKey, oldValue) -- revert changes, it will call onElementDataChange event, but will fail (stop) on first condition - because server (not client) forced change

	return true -- all success
end

function onElementDataChangeBasicAC(dataKey, oldValue, newValue) -- the heart of our anti-cheat, which does all the magic security measurements
	if not client then -- check if data is coming from client
		return false -- if it's not, do not go further
	end

	local checkSpecialThing = (dataKey == "special_thing") -- compare whether dataKey matches "special_thing"
	local checkFlagWaving = (dataKey == "flag_waving") -- compare whether dataKey matches "flag_waving"

	if checkSpecialThing then -- if it does, do our security checks
		local invalidElement = (client ~= source) -- verify whether source element is different from player which changed data

		if invalidElement then -- if it's so
			reportAndRevertDataChange(client, source, dataKey, oldValue, newValue) -- revert data change, because "special_thing" can only be set for player himself
		end
	end

	if checkFlagWaving then -- if it does, do our security checks
		local playerVehicle = getPedOccupiedVehicle(client) -- get player's current vehicle
		local invalidVehicle = (playerVehicle ~= source) -- verify whether source element is different from player's vehicle

		if invalidVehicle then -- if it's so
			reportAndRevertDataChange(client, source, dataKey, oldValue, newValue) -- revert data change, because "flag_waving" can only be set for player's own vehicle
		end
	end
end
addEventHandler("onElementDataChange", root, onElementDataChangeBasicAC)
Click to collapse [-]
Server

Example of comfortable advanced element data anti-cheat.

--[[
	For maximum security set punishPlayerOnDetect, punishmentBan, allowOnlyProtectedKeys to true (as per default configuration)
	If allowOnlyProtectedKeys is enabled, do not forget to add every client-side element data key to protectedKeys table - otherwise you will face false-positives

	Anti-cheat (handleDataChange) table structure and it's options:

	["keyName"] = { -- name of key which would be protected
		onlyForPlayerHimself = true, -- enabling this (true) will make sure that this element data key can only be set on player who synced it (ignores onlyForOwnPlayerVeh and allowForElements), use false/nil to disable this
		onlyForOwnPlayerVeh = false, -- enabling this (true) will make sure that this element data key can only be set on player's current vehicle who synced it (ignores allowForElements), use false/nil to disable this
		allowForElements = { -- restrict this key for certain element type(s), set to false/nil or leave it empty to not check this (full list of element types: https://wiki.multitheftauto.com/wiki/GetElementsByType)
			["player"] = true,
			["ped"] = true,
			["vehicle"] = true,
			["object"] = true,
		},
		allowedDataTypes = { -- restrict this key for certain value type(s), set to false/nil or leave it empty to not check this
			["string"] = true,
			["number"] = true,
			["table"] = true,
			["boolean"] = true,
			["nil"] = true,
		},
		allowedStringLength = {1, 32}, -- if value is a string, then it's length must be in between (min-max), set it to false/nil to not check string length - do note that allowedDataTypes must contain ["string"] = true
		allowedTableLength = {1, 64}, -- if value is a table, then it's length must be in between (min-max), set it to false/nil to not check table length - do note that allowedDataTypes must contain ["table"] = true
		allowedNumberRange = {1, 128}, -- if value is a number, then it must be in between (min-max), set it to false/nil to not check number range - do note that allowedDataTypes must contain ["number"] = true
	}
]]

local punishPlayerOnDetect = true -- should player be punished upon detection (make sure that resource which runs this code has admin rights)
local punishmentBan = true -- only relevant if punishPlayerOnDetect is set to true; use true for ban or false for kick
local punishmentReason = "Altering element data" -- only relevant if punishPlayerOnDetect is set to true; reason which would be shown to punished player
local punishedBy = "Console" -- only relevant if punishPlayerOnDetect is set to true; who was responsible for punishing, as well shown to punished player
local banByIP = false -- only relevant if punishPlayerOnDetect and punishmentBan is set to true; banning by IP nowadays is not recommended (...)
local banByUsername = false -- community username - legacy thing, hence is set to false and should stay like that
local banBySerial = true -- only relevant if punishPlayerOnDetect and punishmentBan is set to true; (...) if there is a player serial to use instead
local banTime = 0 -- only relevant if punishPlayerOnDetect and punishmentBan is set to true; time in seconds, 0 for permanent
local allowOnlyProtectedKeys = true -- disallow (remove by using removeElementData) every element data besides those given in protectedKeys table; in case someone wanted to flood server with garbage keys, which would be kept in memory until server restart or manual remove - setElementData(source, key, nil) won't remove it; it has to be removeElementData
local protectedKeys = {
	["vehicleNumber"] = { -- we want vehicleNumber to be set only on vehicles, with stricte numbers in range of 1-100
		allowForElements = {
			["vehicle"] = true,
		},
		allowedDataTypes = {
			["number"] = true,
		},
		allowedNumberRange = {1, 100},
	},
	["personalVehData"] = { -- we want be able to set personalVehData only on current vehicle, also it should be a string with length between 1-24
		onlyForOwnPlayerVeh = true,
		allowedDataTypes = {
			["string"] = true,
		},
		allowedStringLength = {1, 24},
	},
	-- perform security checks on keys stored in this table, in a convenient way
}

local function reportAndRevertDataChange(clientElement, sourceElement, dataKey, oldValue, newValue, failReason, forceRemove) -- helper function to log and revert changes
	local logClient = inspect(clientElement) -- in-depth view of player which forced element data sync
	local logSerial = getPlayerSerial(clientElement) or "N/A" -- client serial, or "N/A" if not possible, for some reason
	local logSource = inspect(sourceElement) -- in-depth view of element which received data
	local logOldValue = inspect(oldValue) -- in-depth view of old value
	local logNewValue = inspect(newValue) -- in-depth view of new value
	local logText = -- fill our report with data
		"*\n"..
		"Detected element data abnormality:\n"..
		"Client: "..logClient.."\n"..
		"Client serial: "..logSerial.."\n"..
		"Source: "..logSource.."\n"..
		"Data key: "..dataKey.."\n"..
		"Old data value: "..logOldValue.."\n"..
		"New data value: "..logNewValue.."\n"..
		"Fail reason: "..failReason.."\n"..
		"*"

	local debugLevel = 4 -- this debug level allows to hide INFO: prefix, and use custom colors
	local debugR = 255 -- debug message - red color
	local debugG = 127 -- debug message - green color
	local debugB = 0 -- debug message - blue color

	outputDebugString(logText, debugLevel, debugR, debugG, debugB) -- print it to debug

	if forceRemove then -- we don't want this element data key to exist at all
		removeElementData(sourceElement, dataKey) -- disintegrate it

		return true -- data was removed, we don't need to bring it back
	end

	setElementData(sourceElement, dataKey, oldValue) -- revert changes, it will call onElementDataChange event, but will fail (stop) on first condition - because server (not client) forced change

	return true -- return success
end

local function handleDataChange(clientElement, sourceElement, dataKey, dataValue) -- the heart of our anti-cheat, which does all the magic security measurements, returns true if there was no altering from client (based on data from protectedKeys), false otherwise
	local protectedKey = protectedKeys[dataKey] -- look up whether key changed is stored in protectedKeys table

	if not protectedKey then -- if it's not

		if allowOnlyProtectedKeys then -- if we don't want garbage keys
			return false, "Key isn't present in protectedKeys", true -- let reportAndRevertDataChange know that this key should be removed
		end

		return true -- this key isn't protected, let it through
	end

	local onlyForPlayerHimself = protectedKey.onlyForPlayerHimself -- if key has "self-set" lock
	local onlyForOwnPlayerVeh = protectedKey.onlyForOwnPlayerVeh -- if key has "self-vehicle" lock
	local allowForElements = protectedKey.allowForElements -- if key has element type check
	local allowedDataTypes = protectedKey.allowedDataTypes -- if key has allowed data type check

	if onlyForPlayerHimself then -- if "self-set" lock is active
		local matchingElement = (clientElement == sourceElement) -- verify whether player who set data is equal to element which received data

		if not matchingElement then -- if it's not matching
			return false, "Can only set on player himself" -- return failure
		end
	end

	if onlyForOwnPlayerVeh then -- if "self-vehicle" lock is active
		local playerVehicle = getPedOccupiedVehicle(clientElement) -- get current vehicle of player which set data
		local matchingVehicle = (playerVehicle == sourceElement) -- check whether it matches the one which received data

		if not matchingVehicle then -- if it doesn't match
			return false, "Can only set on player's own vehicle" -- return failure
		end
	end

	if allowForElements then -- check if it's one of them
		local elementType = getElementType(sourceElement) -- get type of element whose data changed
		local matchingElementType = allowForElements[elementType] -- verify whether it's allowed

		if not matchingElementType then -- this isn't matching
			return false, "Invalid element type" -- consider it failure
		end
	end

	if allowedDataTypes then -- if there's allowed data types
		local valueType = type(dataValue) -- get data type of value
		local matchingType = allowedDataTypes[valueType] -- check if it's one of allowed

		if not matchingType then -- if it's not then
			return false, "Invalid data type" -- report failure
		end

		local allowedStringLength = protectedKey.allowedStringLength -- if key has specified string length check
		local dataString = (valueType == "string") -- make sure it's a string

		if allowedStringLength and dataString then -- if we should check string length
			local minLength = allowedStringLength[1] -- retrieve min length
			local maxLength = allowedStringLength[2] -- retrieve max length
			local stringLength = utf8.len(dataValue) -- get length of data string
			local matchingLength = (stringLength >= minLength) and (stringLength <= maxLength) -- compare whether value fits in between

			if not matchingLength then -- if it doesn't
				return false, "Invalid string length (must be between "..minLength.."-"..maxLength..")" -- mark it as incorrect
			end
		end

		local allowedTableLength = protectedKey.allowedTableLength -- if key has table length check
		local dataTable = (valueType == "table") -- make sure it's a table

		if allowedTableLength and dataTable then -- if we should check table length
			local minLength = allowedTableLength[1] -- retrieve min length
			local maxLength = allowedTableLength[2] -- retrieve max length
			local matchingLength = true -- store it initally as true, later we will compare it inside loop, to not waste CPU on travelling through whole table
			local tableLength = 0 -- store initial table length

			for _, _ in pairs(dataValue) do -- loop through whole table
				tableLength = (tableLength + 1) -- add + 1 on each table entry
				matchingLength = (tableLength >= minLength) and (tableLength <= maxLength) -- check whether condition is true on each table entry

				if not matchingLength then -- it's not
					break -- break the loop (due of condition above being unworthy, it makes no point to count further)
				end
			end

			if not matchingLength then -- this table doesn't match requirements
				return false, "Invalid table length (must be between "..minLength.."-"..maxLength..")" -- mark it as incorrect
			end
		end

		local allowedNumberRange = protectedKey.allowedNumberRange -- if key has allowed number range check
		local dataNumber = (valueType == "number") -- make sure it's a number

		if allowedNumberRange and dataNumber then -- if we should check number range
			local minRange = allowedNumberRange[1] -- retrieve min number range
			local maxRange = allowedNumberRange[2] -- retrieve max number range
			local matchingRange = (dataValue >= minRange) and (dataValue <= maxRange) -- compare whether value fits in between

			if not matchingRange then -- if it doesn't
				return false, "Invalid number range (must be between "..minRange.."-"..maxRange..")" -- mark it as incorrect
			end
		end
	end

	return true -- security checks passed, we are all clear with this data key
end

function onElementDataChangeAdvancedAC(dataKey, oldValue, newValue) -- this event makes use of handleDataChange, the code was split for better readability
	if not client then -- check if data is coming from client
		return false -- if it's not, do not continue
	end

	local approvedChange, failReason, forceRemove = handleDataChange(client, source, dataKey, newValue) -- run our security checks

	if approvedChange then -- it's all cool and good
		return false -- we don't need further action
	end

	reportAndRevertDataChange(client, source, dataKey, oldValue, newValue, failReason, forceRemove) -- do it!

	if not punishPlayerOnDetect then -- we don't want to punish player for some reason
		return false -- so stop here
	end
		
	if punishmentBan then -- if it's ban
		banPlayer(client, banByIP, banByUsername, banBySerial, punishedBy, punishmentReason, banTime) -- remove his presence from server
	else -- otherwise
		kickPlayer(client, punishedBy, punishmentReason) -- simply kick player out of server
	end
end
addEventHandler("onElementDataChange", root, onElementDataChangeAdvancedAC)

Securing triggerServerEvent

  • All parameters including source can be faked and should not be trusted.
  • Global variable client can be trusted.
  • Admin styled events should be verifying player (client) ACL rights, either by isObjectInACLGroup or hasObjectPermissionTo.

Click to collapse [-]
Server

Example of comfortable anti-cheat function designed for events, used for data validation for both normal, and admin events which are called from client-side.

--[[
	For maximum security set punishPlayerOnDetect, punishmentBan to true (as per default configuration)

	Anti-cheat (processServerEventData) table structure and it's options:

	checkACLGroup = { -- check whether player who called event belongs to at least one group below, set to false/nil to not check this
		"Admin",
	},
	checkPermissions = { -- check whether player who called event has permission to at least one thing below, set to false/nil to not check this
		"function.kickPlayer",
	},
	checkEventData = {
		{
			debugData = "source", -- optional details for report shown in debug message
			eventData = source, -- data we want to verify
			equalTo = client, -- compare whether eventData == equalTo
			allowedElements = { -- restrict eventData to be certain element type(s), set to false/nil or leave it empty to not check this (full list of element types: https://wiki.multitheftauto.com/wiki/GetElementsByType)
				["player"] = true,
				["ped"] = true,
				["vehicle"] = true,
				["object"] = true,
			},
			allowedDataTypes = { -- restrict eventData to be certain value type(s), set to false/nil or leave it empty to not check this
				["string"] = true,
				["number"] = true,
				["table"] = true,
				["boolean"] = true,
				["nil"] = true,
			},
			allowedStringLength = {1, 32}, -- if eventData is a string, then it's length must be in between (min-max), set it to false/nil to not check string length - do note that allowedDataTypes must contain ["string"] = true
			allowedTableLength = {1, 64}, -- if eventData is a table, then it's length must be in between (min-max), set it to false/nil to not check table length - do note that allowedDataTypes must contain ["table"] = true
			allowedNumberRange = {1, 128}, -- if eventData is a number, then it must be in between (min-max), set it to false/nil to not check number range - do note that allowedDataTypes must contain ["number"] = true
		},
	},
]]

local punishPlayerOnDetect = true -- should player be punished upon detection (make sure that resource which runs this code has admin rights)
local punishmentBan = true -- only relevant if punishPlayerOnDetect is set to true; use true for ban or false for kick
local punishmentReason = "Altering server event data" -- only relevant if punishPlayerOnDetect is set to true; reason which would be shown to punished player
local punishedBy = "Console" -- only relevant if punishPlayerOnDetect is set to true; who was responsible for punishing, as well shown to punished player
local banByIP = false -- only relevant if punishPlayerOnDetect and punishmentBan is set to true; banning by IP nowadays is not recommended (...)
local banByUsername = false -- community username - legacy thing, hence is set to false and should stay like that
local banBySerial = true -- only relevant if punishPlayerOnDetect and punishmentBan is set to true; (...) if there is a player serial to use instead
local banTime = 0 -- only relevant if punishPlayerOnDetect and punishmentBan is set to true; time in seconds, 0 for permanent

function processServerEventData(clientElement, sourceElement, securityChecks) -- the heart of our anti-cheat, which does all the magic security measurements, returns true if there was no altering from client (based on data from securityChecks), false otherwise
	if not securityChecks then -- if we haven't passed any security checks
		return true -- nothing to check, let code go further
	end
	
	if not clientElement then -- if client variable isn't available for some reason (although it should never happen)
		return false, "Client variable not present", true -- stop code execution, skip punishment
	end

	local checkACLGroup = securityChecks.checkACLGroup -- if there's any ACL groups to check
	local checkPermissions = securityChecks.checkPermissions -- if there's any permissions to check
	local checkEventData = securityChecks.checkEventData -- if there's any data checks

	if checkACLGroup then -- let's check player ACL groups
		local playerAccount = getPlayerAccount(clientElement) -- get current account of player
		local guestAccount = isGuestAccount(playerAccount) -- if account is guest (meaning player is not logged in)

		if guestAccount then -- it's the case
			return false, "Can't retrieve player login - guest account", true -- stop code execution, skip punishment
		end

		local accountName = getAccountName(playerAccount) -- get name of player's current account
		local aclString = "user."..accountName -- format it for further use in isObjectInACLGroup function

		for groupID = 1, #checkACLGroup do -- iterate over table of given groups
			local groupName = checkACLGroup[groupID] -- get each group name
			local aclGroup = aclGetGroup(groupName) -- check if such group exists

			if not aclGroup then -- it doesn't
				return false, "ACL group '"..groupName.."' is missing", true -- stop code execution, skip punishment
			end

			local playerInACLGroup = isObjectInACLGroup(aclString, aclGroup) -- check if player belong to the group

			if playerInACLGroup then -- yep, it's the case
				return true -- so it's a success
			end
		end

		return false, "Player doesn't belong to any given ACL group", true -- stop code execution, skip punishment
	end

	if checkPermissions then -- check if player has at least one desired permission
		local allowedByDefault = false -- does he have access by default

		for permissionID = 1, #checkPermissions do -- iterate over all permissions
			local permissionName = checkPermissions[permissionID] -- get permission name
			local hasPermission = hasObjectPermissionTo(clientElement, permissionName, allowedByDefault) -- check whether player is allowed to perform certain action

			if hasPermission then -- if player has access
				return true -- one is available (and enough), server won't bother to check others (as return keywords also breaks loop)
			end
		end

		return false, "Not enough permissions", true -- stop code execution, skip punishment
	end

	if checkEventData then -- if there is some data to verify

		for dataID = 1, #checkEventData do -- iterate over each of data
			local dataToCheck = checkEventData[dataID] -- get each data set
			local eventData = dataToCheck.eventData -- this is the one we'll be verifying
			local equalTo = dataToCheck.equalTo -- we want to compare whether eventData == equalTo
			local allowedElements = dataToCheck.allowedElements -- check whether is element, and whether belongs to certain element types
			local allowedDataTypes = dataToCheck.allowedDataTypes -- do we restrict data to be certain type?
			local debugData = dataToCheck.debugData -- additional helper data
			local debugText = debugData and " ("..debugData..")" or "" -- if it's present, format it nicely

			if equalTo then -- equal check exists
				local matchingData = (eventData == equalTo) -- compare whether those two values are equal

				if not matchingData then -- they aren't
					return false, "Data isn't equal @ argument "..dataID..debugText -- report failure
				end
			end

			if allowedElements then -- we do check whether is an element, and belongs to at least one given in the list
				local validElement = isElement(eventData) -- check if it's actual element

				if not validElement then -- it's not
					return false, "Data isn't element @ argument "..dataID..debugText -- failure here
				end

				local elementType = getElementType(eventData) -- it's element, so we want to know it's type
				local matchingElementType = allowedElements[elementType] -- verify whether it's allowed

				if not matchingElementType then -- it's not allowed
					return false, "Invalid element type @ argument "..dataID..debugText -- return failure
				end
			end

			if allowedDataTypes then -- let's check allowed data types
				local dataType = type(eventData) -- get data type
				local matchingType = allowedDataTypes[dataType] -- verify whether it's allowed

				if not matchingType then -- it isn't
					return false, "Invalid data type @ argument "..dataID..debugText -- it's failure
				end

				local allowedStringLength = dataToCheck.allowedStringLength -- if data has string length check
				local dataString = (dataType == "string") -- make sure it's a string

				if allowedStringLength and dataString then -- if we should check string length
					local minLength = allowedStringLength[1] -- retrieve min length
					local maxLength = allowedStringLength[2] -- retrieve max length
					local stringLength = utf8.len(eventData) -- get length of data string
					local matchingLength = (stringLength >= minLength) and (stringLength <= maxLength) -- compare whether value fits in between

					if not matchingLength then -- if it doesn't
						return false, "Invalid string length (must be between "..minLength.."-"..maxLength..") @ argument "..dataID..debugText -- it doesn't, failure
					end
				end

				local allowedTableLength = dataToCheck.allowedTableLength -- if data has table length check
				local dataTable = (dataType == "table") -- make sure it's a table

				if allowedTableLength and dataTable then -- if we should check table length
					local minLength = allowedTableLength[1] -- retrieve min length
					local maxLength = allowedTableLength[2] -- retrieve max length
					local matchingLength = true -- store it initally as true, later we will compare it inside loop, to not waste CPU on travelling through whole table
					local tableLength = 0 -- store initial table length

					for _, _ in pairs(eventData) do -- loop through whole table
						tableLength = (tableLength + 1) -- add + 1 on each table entry
						matchingLength = (tableLength >= minLength) and (tableLength <= maxLength) -- check whether condition is true on each table entry

						if not matchingLength then -- it's not
							break -- break the loop (due of condition above being unworthy, it makes no point to count further)
						end
					end

					if not matchingLength then -- this table doesn't match requirements
						return false, "Invalid table length (must be between "..minLength.."-"..maxLength..") @ argument "..dataID..debugText -- it doesn't, failure
					end
				end

				local allowedNumberRange = dataToCheck.allowedNumberRange -- if data has number range check
				local dataNumber = (dataType == "number") -- make sure it's a number

				if allowedNumberRange and dataNumber then -- if we should check number range
					local minRange = allowedNumberRange[1] -- retrieve min number range
					local maxRange = allowedNumberRange[2] -- retrieve max number range
					local matchingRange = (eventData >= minRange) and (eventData <= maxRange) -- compare whether value fits in between

					if not matchingRange then -- if it doesn't
						return false, "Invalid number range (must be between "..minRange.."-"..maxRange..") @ argument "..dataID..debugText -- -- it doesn't, failure
					end
				end
			end
		end
	end

	return true -- security checks passed, we are all clear with this event call
end

function reportAndHandleEventAbnormality(clientElement, sourceElement, serverEvent, failReason, skipPunishment) -- helper function to log and handle accidents
	local logClient = inspect(clientElement) -- in-depth view player which called event
	local logSerial = getPlayerSerial(clientElement) or "N/A" -- client serial, or "N/A" if not possible, for some reason
	local logSource = inspect(sourceElement) -- in-depth view of source element
	local logText = -- fill our report with data
		"*\n"..
		"Detected event abnormality:\n"..
		"Client: "..logClient.."\n"..
		"Client serial: "..logSerial.."\n"..
		"Source: "..logSource.."\n"..
		"Event: "..serverEvent.."\n"..
		"Reason: "..failReason.."\n"..
		"*"

	local debugLevel = 4 -- this debug level allows to hide INFO: prefix, and use custom colors
	local debugR = 255 -- debug message - red color
	local debugG = 127 -- debug message - green color
	local debugB = 0 -- debug message - blue color

	outputDebugString(logText, debugLevel, debugR, debugG, debugB) -- print it to debug

	if not punishPlayerOnDetect or skipPunishment then -- we don't want to punish player for some reason
		return true -- stop here
	end
		
	if punishmentBan then -- if it's ban
		banPlayer(clientElement, banByIP, banByUsername, banBySerial, punishedBy, punishmentReason, banTime) -- remove his presence from server
	else -- otherwise
		kickPlayer(clientElement, punishedBy, punishmentReason) -- simply kick player out of server
	end

	return true -- all done, report success
end

function onServerEvent(clientData)
	--[[
		Assume this server event (function) is called in such way:

		local dataToPass = 10

		triggerServerEvent("onServerEvent", localPlayer, dataToPass)
	]]

	local shouldProcessServerCode, failReason, skipPunishment = processServerEventData(
		client, -- client element - responsible for calling event
		source, -- source element - passed in triggerServerEvent (as 2nd argument)
		{
			checkEventData = { -- we want to verify everything what comes from client
				{
					eventData = source, -- first to check, source variable
					equalTo = client, -- we want to check whether it matches player who called event
					debugData = "source", -- helper details which would be shown in report
				},
				{
					eventData = clientData, -- let's check the data which client sent to us
					allowedDataTypes = {
						["number"] = true, -- we want it to be only number
					},
					allowedNumberRange = {1, 100}, -- in range of 1 to 100
					debugData = "clientData", -- if something goes wrong, let server know where (it will appear in debug report)
				},
			},
		}
	)

	if not shouldProcessServerCode then -- something isn't right, no green light for processing code behind this scope
		reportAndHandleEventAbnormality(client, source, eventName, failReason, skipPunishment) -- report accident, and handle this player

		return false -- stop code execution
	end

	-- do code as usual
end
addEvent("onServerEvent", true)
addEventHandler("onServerEvent", root, onServerEvent)

function onServerAdminEvent(playerToBan)
	--[[
		Assume this server admin event (function) is called in such way:

		local playerToBan = getPlayerFromName("playerToBan")

		triggerServerEvent("onServerAdminEvent", localPlayer, playerToBan)
	]]

	local shouldProcessServerCode, failReason, skipPunishment = processServerEventData(
		client, -- client element - responsible for calling event
		source, -- source element - passed in triggerServerEvent (as 2nd argument)
		{
			checkACLGroup = { -- we need to check whether player who called event belongs to ACL groups
				"Admin", -- in this case admin group
			},
			checkEventData = { -- we want to verify everything what comes from client
				{
					eventData = source, -- first to check, source variable
					equalTo = client, -- we want to check whether it matches player who called event
					debugData = "source", -- helper details which would be shown in report
				},
				{
					eventData = playerToBan, -- let's check the data which client sent to us
					allowedDataTypes = {
						["player"] = true, -- we want it to be player
					},
					debugData = "playerToBan", -- if something goes wrong, let server know where (it will appear in debug report)
				},
			},
		}
	)

	if not shouldProcessServerCode then -- something isn't right, no green light for processing code behind this scope
		reportAndHandleEventAbnormality(client, source, eventName, failReason, skipPunishment) -- report accident, and handle this player

		return false -- stop code execution
	end

	-- do code as usual
end
addEvent("onServerAdminEvent", true)
addEventHandler("onServerAdminEvent", root, onServerAdminEvent)

Securing server-only events

  • It is very important to disable remote triggering ability in addEvent, to prevent calling server-side only events from client-side.
  • Admin styled events should be verifying player ACL rights, either by isObjectInACLGroup or hasObjectPermissionTo.

Click to collapse [-]
Server

This example shows how you should make event server-side only.

function onServerSideOnlyEvent()
	-- do some server-side stuff
end
addEvent("onServerSideOnlyEvent", false) -- set second argument (allowRemoteTriger) to false, so it can't be called from client
addEventHandler("onServerSideOnlyEvent", root, onServerSideOnlyEvent) -- associate our event with function onServerSideOnlyEvent