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.
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 called event 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 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)
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 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 = "Malicious altering of 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 called event 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.
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
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:
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)
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)