AttorneyOnline / tsuserver3

An Attorney Online server.
GNU Affero General Public License v3.0
25 stars 51 forks source link

Banning someone for "1 week" results in a ban lasting 69 years #169

Closed in1tiate closed 3 years ago

in1tiate commented 3 years ago

via Lernos#4694 on Discord:

Seems like some serious stuff this time. Ban durations are wrong on tsuserver3. Tested /ban IPID "reason" "1 minute" and it banned the person for 2 days. "3 minutes" gave us a week (around 7 days and 12 hours). I'm not sure if it's something to do with the database or datetime.

in1tiate commented 3 years ago

The offending code, in server/commands/admin.py:

elif len(args) == 3:
    ipid = _convert_ipid_to_int(args[0])
    reason = args[1]
    duration = args[2]
    ban_duration = parse(str(duration), granularity='hours')

    if duration is None:
        raise ArgumentError('Invalid ban duration.')
    elif 'perma' in duration.lower():
        unban_date = None
    else:
        if ban_duration is not None:
            unban_date = arrow.get().shift(hours=ban_duration).datetime
        else:
            raise ArgumentError(f'{duration} is an invalid ban duration')

Multiple things to note here. Firstly, granularity='hours' does nothing at all, according to the pytimeparse code (via pytimeparse/timeparse.py:

if granularity == 'minutes':
  mdict = _interpret_as_minutes(sval, mdict)

"granularity" only accepts 'minutes', and all other input is discarded. This is important, because I assume the original author of this code assumed that granularity='hours' meant that pytimeparse would return the parsed time to them in hours. This is not the case; pytimeparse always returns the time in seconds.

Secondly, the time is interpreted by arrow as if it were in hours rather than seconds, resulting in:

I: 1 minute O: 60 hours (roughly 2 days) I: 3 minutes O: 180 hours (exactly 1 week and 12 hours) I: 1 week O: 604,800 hours (69 calendar years)

This would seem to beg the question of how in the absolute fuck no one has noticed this until now.

in1tiate commented 3 years ago

Appears to have been caused by https://github.com/AttorneyOnline/tsuserver3/pull/154