Script security

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 hacked clients

To ensure minimum damage when a player with a hacked client 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.
• 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 them with no participation of client-side.

Validating client 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 = getPlayerName(clientElement) -- name of player which forced element data sync
	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"..
		"Possible rogue client!\n"..
		"clientElement: "..logClient.."\n"..
		"sourceElement: "..logSource.."\n"..
		"dataKey: "..dataKey.."\n"..
		"oldValue: "..logOldValue.."\n"..
		"newValue: "..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)
	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 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
		allowedNumberRange = {1, 100}, -- 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 = getPlayerName(clientElement) -- name of player which forced element data sync
	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"..
		"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
end

local function handleDataChange(clientElement, sourceElement, dataKey, dataValue) -- this function 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, "Not allowed 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, "Not allowed 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 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)
	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)

Validating client triggerServerEvent

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

---

Click to collapse [-]

Server

Example validation of event parameters

addEvent("onRaiseTheRoof", true)
addEventHandler("onRaiseTheRoof", resourceRoot,
    function(arg1, arg2)
        -- Check data is coming from a client
        if client then

            -- The source for this event is always 'resourceRoot'
            if source ~= resourceRoot then
                reportNaughtyness( eventName, client, "source" )
                return
            end

            -- arg1 should be the player
            if arg1 ~= client then
                reportNaughtyness( eventName, client, "arg1" )
                return
            end

            -- arg2 should be between 1 and 100
            if arg2 < 1 or arg2 >100 then
                reportNaughtyness( eventName, client, "arg2" )
                return
            end
        end

        --
        -- Do usual code for 'onRaiseTheRoof'
        --
    end
)

-- Helper function to log illegal things
function reportNaughtyness( eventName, client, reason )
    -- Report
    outputConsole( "Possible rogue client!"
            .. " client:" .. tostring(getPlayerName(client))
            .. " eventName:" .. tostring(eventName)
            .. " reason:" .. tostring(reason)
            )
end

---

Click to collapse [-]

Server

Example validation of event parameters for admin style event

addEvent("onRequestBanPlayer", true)
addEventHandler("onRequestBanPlayer", resourceRoot,
    function(arg1, arg2)
        -- Check data is coming from a client
        if client then

            -- Check client has permission to do the deed
            if not canPlayerBan(client) then
                reportNaughtyness( eventName, client, "client" )
                return
            end

        end

        --
        -- Do usual code for 'onRequestBanPlayer'
        --
    end
)

Validating client ACL rights

In server side event handlers, always check that client global variable (which refers to player which truthfully called event) has permission before doing anything. This will stop hackers and script bugs from destroying your server.

Firstly, and example of BAD, INSECURE script:

Click to collapse [-]

Server

BAD, INSECURE script:

-- NO PROBLEM HERE: Command 'showgui' will only show the gui for admins
function showGui(player)
    if isObjectInGroup( player, "admin" ) then
        -- Only trigger client GUI for admins
        triggerEvent( player, "onShowGui", resourceRoot )
    end
end
addCommandHandler("showgui", showGui)

-- MISTAKE HERE: Incorrectly assume onMyGuiCommand can only be called by admins
-- Script bugs and hackers mean this function can be called by anyone
function onMyGuiCommand(name)
    aclGroupAddObject(aclGetGroup("admin"), "user."..name)
    outputServerLog( "DO NOT USE THIS IS WRONG " )
end
addEventHandler("onMyGuiCommand", resourceRoot, onMyGuiCommand)

onMyGuiCommand does not check if the sending player has permission, and is depending on code having no bugs, and no hackers in the world. Simple to fix by adding a client global variable check at the start. (Note: checking source will not fix the problem - It has to be client)

Click to collapse [-]

Server

Fixed onMyGuiCommand:

function onMyGuiCommand(name)
    -- Check 'client' really does have permission
    local clientAccountName = getAccountName( getPlayerAccount( client ) )
    if not isObjectInACLGroup ( "user." .. clientAccountName, aclGetGroup ( "Admin" ) ) ) then
        outputServerLog( "ACCESS VIOLATION by " .. getPlayerName( client ) )
        return
    end
    -- Client has permission now
    aclGroupAddObject(aclGetGroup("admin"), "user."..name)
end
addEventHandler("onMyGuiCommand", resourceRoot, onMyGuiCommand)