Ban Exploit: SQL banning offline players ignores IP and Computer ID

SpadesNeil commented 8 years ago

Brief description of the issue

This is exploited maliciously by griefers to ban evade.

What you expected to happen

When you ban an offline player whose mob is SSD, for it to grab the lastknownIP and computerID vars, and include them in the ban.

What actually happened

Only the ckey is banned.

Steps to reproduce

Attempt to ban someone who disconnects before you can ban them, leaving their mob SSD.
Check the unban panel.
Additional info:
- Server Revision: https://github.com/PolarisSS13/Polaris/commit/85d1ef3062076d89d91c00236bdf579698389d82
- Anything else you may wish to add: Ban system works normally when the player is still online when banned.

SpadesNeil commented 7 years ago

I feel like @Leshana fixed this in Vorestation but I am not sure. Maybe @Arokha fixed it? Maybe no one fixed it and I am actually crazy.

Neerti commented 7 years ago

Porting and finishing #2644 would probably put a stop to this for good, if it's still occurring.

skull132 commented 7 years ago

@SpadesNeil It's an easy fix, I did it on Aurora years ago.

There's a query in modules/admin/DB ban/functions.dm on line ~40, which checks if a player is actually logged on the database. It does so by select the id of said player. Just modify that query to also pull the computerid and ip. And shove that into the if(query.NextRow()) clause, so it replaces a missing CID/IP with the query result.

Original:

    var/DBQuery/query = dbcon.NewQuery("SELECT id FROM erro_player WHERE ckey = '[ckey]'")
    query.Execute()
    var/validckey = 0
    if(query.NextRow())
        validckey = 1
    if(!validckey)
        if(!banned_mob || (banned_mob && !IsGuestKey(banned_mob.key)))
            message_admins("<font color='red'>[key_name_admin(usr)] attempted to ban [ckey], but [ckey] has not been seen yet. Please only ban actual players.</font>",1)
            return

Fixed version:

    var/DBQuery/query = dbcon.NewQuery("SELECT id, computerid, ip FROM erro_player WHERE ckey = '[ckey]'")
    query.Execute()
    var/validckey = 0
    if(query.NextRow())
        validckey = 1
        // Stop relying on manual entry, use the database.
        if (!computerid)
            computerid = query.item[2]
        if (!ip)
            ip = query.item[3]
    if(!validckey)
        if(!banned_mob || (banned_mob && !IsGuestKey(banned_mob.key)))
            message_admins("<font color='red'>[key_name_admin(usr)] attempted to ban [ckey], but [ckey] has not been seen yet. Please only ban actual players.</font>",1)
            return

PolarisSS13 / Polaris