From ecd2622a7505f74e764e08a9d3281ebeff1fc62e Mon Sep 17 00:00:00 2001 From: Kyle Spier-Swenson Date: Mon, 13 Jan 2020 14:23:32 -0800 Subject: [PATCH] Don't recheck connected clients in isbanned() (and other isbanned() dos mitigations) (#48583) dos mitigation that likely won't fully work but let's see anyways. backstory: byond's ban system will automatically ban world/Topic flooders. Sadly updating byond's internal ban list causes isbanned() to get called on all connected clients. Also sadly: it does this every time it sees the world/Topic flood on the same ip, causing every connected client gets checked repeatedly for every flood message world/Topic sees. --- code/modules/admin/IsBanned.dm | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/code/modules/admin/IsBanned.dm b/code/modules/admin/IsBanned.dm index f3410d06e30..90d792a2864 100644 --- a/code/modules/admin/IsBanned.dm +++ b/code/modules/admin/IsBanned.dm @@ -13,10 +13,17 @@ return FALSE log_access("Failed Login (invalid data): [key] [address]-[computer_id]") return list("reason"="invalid login data", "desc"="Error: Could not check ban status, Please try again. Error message: Your computer provided invalid or blank information to the server on connection (byond username, IP, and Computer ID.) Provided information for reference: Username:'[key]' IP:'[address]' Computer ID:'[computer_id]'. (If you continue to get this error, please restart byond or contact byond support.)") - + + if (type == "world") + return ..() //shunt world topic banchecks to purely to byond's internal ban system + var/admin = FALSE var/ckey = ckey(key) - + + var/client/C = GLOB.directory[ckey] + if (C && ckey == C.ckey && computer_id == C.computer_id && address == C.address) + return //don't recheck connected clients. + //IsBanned can get re-called on a user in certain situations, this prevents that leading to repeated messages to admins. var/static/list/checkedckeys = list() //magic voodo to check for a key in a list while also adding that key to the list without having to do two associated lookups @@ -25,8 +32,6 @@ if(GLOB.admin_datums[ckey] || GLOB.deadmins[ckey]) admin = TRUE - var/client/C = GLOB.directory[ckey] - //Whitelist if(!real_bans_only && !C && CONFIG_GET(flag/usewhitelist))