Wrapper around net.Incoming to prevent clients from spamming bad net messages
7 replies, posted
I've been hearing from server owners that they are being attacked by clients spamming bad error messages to the server which causes lua errors. This fills up the error log and spams the server console causing lag.
[lua]
if SERVER then
net.old_incoming = net.old_incoming or net.Incoming
local last_receive_error = {}
local last_print_suppress = {}
local SysTime = SysTime
local RealTime = RealTime
function net.Incoming(length, ply, ...)
if ... then
ErrorNoHalt("net.Incoming called with more than 2 arguments? skipping pcall protection")
return net.old_incoming(length, ply, ...)
end
if last_receive_error[ply] and last_receive_error[ply] + 1 > SysTime() then
if not last_print_suppress[ply] or last_print_suppress[ply] < RealTime() then
print("last message from ", ply, "errored less than a second ago, supressing net message")
last_print_suppress[ply] = RealTime() + 1
end
return
end
local ret = {
xpcall(
net.old_incoming,
function(msg)
ErrorNoHalt(debug.traceback("net message " .. id .. " from " .. tostring(ply) .. " errored: "))
end,
length,
ply
)
}
if ret[1] then
return unpack(ret, 2)
end
last_receive_error[ply] = SysTime()
end
end
[/lua]
This only prevents clients from error spamming. Maybe we can also have something in the net library that tells us how many times a net message is allowed to be sent per second? That would help a lot.
Something like net.Receive("foobar", function() end, 0.5) which would make it so you can only receive this net message once every 0.5 seconds.
The real solution is that we developers fix our shit, but I would argue that this is really difficult when we use net.ReadTable and net.WriteTable. The best practice is to send and receive messages that are in a predictable length and validate each type. This is fine for small net messages but for bigger ones it becomes problematic.
There's also that we can do mistakes so it's better to protect against future exploits than nothing.
I made this method to easily prevent things like net message and console command spamming:
local entmeta = FindMetaTable("Entity")
function entmeta:RateLimit(key, delay)
local id = "ratelimit_" .. key
if self[id] then
if self[id] + delay > CurTime() then return true end
end
self[id] = CurTime()
return false
end
Just use it like:
net.Receive("something", function(_, ply)
if ply:RateLimit("someid", 0.5) then return end
--rest here
end)
So it would be nice to have something like this but per net message instead. Maybe it can be done in engine for an earlier exit.
Are you sure using CurTime is safe though? If there's a lot of lag CurTime might not be reliable.
This honestly feels like something that's up to the developer to account for and shouldn't be a parameter for the Receive function.
I'm not sure, maybe RealTime is better?
I would use SysTime as RealTime does not advance during 1 frame.
Developers can do rate limiting but we don't know if this can be done more efficiently in the engine.
-- Net proxy to stop people systematically bombing in-effecient addons etc.
-- Could also prevent bugs in addons spamming the server with net messages?
net = net or {}
--local DEV_SERVER = false
local whitelist = { -- Things which we know aren't exploitable
["84vZBftk"] = true,
["jwGF6GWk"] = true,
["4h4ZMNLQ"] = true,
["Aero"] = true,
["AGit"] = true,
["AGit_Complete"] = true,
["AGit_Request"] = true,
["gm_netmsg"] = true,
["rprint_selectmode"] = true,
}
if CLIENT then -- Assume the client isn't a hacker and the issue is a bug causing them to spam net messages
oSendToServer = oSendToServer or net.SendToServer
local ply = LocalPlayer()
local function netProxy() -- Clientside proxy
local curtime = CurTime()
if ply.netDelay and ply.netDelay > curtime then
if !DEV_SERVER then return else print("You're sending net messages quickly!") end
end
ply.netDelay = curtime + 0.15
oSendToServer()
end
function net.SendToServer()
netProxy()
end
else -- Ensure net messages are not exceding the delay in case they bypassed our clientside detour
oReceive = oReceive or net.Receive
local function netProxy(len, ply, callback, msg) -- Serverside proxy
local curtime = CurTime()
ply.netDelay = ply.netDelay or {}
if ply.netDelay[msg] > curtime and not whitelist[msg] then
print(ply:SteamID() .. ": is sending net message very quickly: " .. (msg or "Unknown!"))
if !DEV_SERVER then return else print(" > Ignoring because server is in development mode.") end
end
ply.netDelay[msg] = curtime + 0.5
callback(len, ply)
end
function net.Receive(msg, callback)
oReceive(msg, function(len, ply) netProxy(len, ply, callback, msg) end)
end
end
You might find this useful. I haven't tested it, but this is something I threw together while back to address a similar issue. It should rate limit all net messages & have a unique cool down for each message server side.
It needs to be loaded early on to work, obviously. Any suggestions for improvement are welcome even though I don't use this anymore :v
Of course, but this should prevent abusing the ones we don't know about.
But I'm honestly not sure how to handle dynamic data. (net.ReadTable and net.WriteTable) The best is probably not to have dynamic data at all. Once you're using net.ReadTable you can just do net.WriteUInt(1337, 8) from client and you'll cause errors on server..
Sorry, you need to Log In to post a reply to this thread.