• 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.