Script security
Jump to navigation
Jump to search
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 the effects of rogue 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.
- When reviewing scripts for possible security holes. Look at any data coming from the client that is being trusted.
A hacked client could send anything, so all client data received by server scripts should be validated before use. Mostly, it will be done either by setElementData or triggerServerEvent.
Validating client setElementData
- All parameters including source can be faked and should not be trusted.
- Global variable client can be trusted.
Click to collapse [-]
ServerExample 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)
local logSource = tostring(sourceElement)
local logOldValue = tostring(oldValue)
local logNewValue = tostring(newValue)
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"..
"======================================="
outputConsole(logText, root) -- 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)
Click to collapse [-]
ServerExample 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 and punishmentBan 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 allowed string range 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)
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 [-]
ServerExample 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 [-]
ServerExample 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 [-]
ServerBAD, 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 [-]
ServerFixed 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)