diff options
author | Himbeer <himbeer@disroot.org> | 2025-05-01 20:34:24 +0200 |
---|---|---|
committer | Himbeer <himbeer@disroot.org> | 2025-05-01 20:39:31 +0200 |
commit | 1fcc619f910c763cebb09fc0d72533a6a6eb4dce (patch) | |
tree | 2dbe2eb34b4479b664a40c8a1e3082836601d66a | |
parent | eaf851280ef74fdcef84698ce51a04770c1b2b39 (diff) |
Previously, a client failing this check would be disconnected,
incorrectly resulting in the username being removed from the playerlist
and enabling another client to log in. The only limit to this was the
number of internal servers accessible to the account. This commit fixes
the bypass by preventing the username from being removed from the list
in error cases.
-rw-r--r-- | process.go | 43 |
1 files changed, 24 insertions, 19 deletions
@@ -79,6 +79,30 @@ func (cc *ClientConn) process(pkt mt.Pkt) { cc.name = cmd.PlayerName cc.logger.SetPrefix(fmt.Sprintf("[%s %s] ", cc.RemoteAddr(), cc.Name())) + playersMu.Lock() + _, ok := players[cc.Name()] + if ok { + cc.Log("<-", "already connected") + ack, _ := cc.SendCmd(&mt.ToCltKick{Reason: mt.AlreadyConnected}) + + select { + case <-cc.Closed(): + case <-ack: + cc.Close() + } + + // Needed so that the username doesn't get removed from + // the player list which would allow other clients to + // bypass this check. + cc.name = "" + + playersMu.Unlock() + return + } + + players[cc.Name()] = struct{}{} + playersMu.Unlock() + if !playerNameChars.MatchString(cmd.PlayerName) { cc.Log("<-", "invalid player name") ack, _ := cc.SendCmd(&mt.ToCltKick{Reason: mt.BadNameChars}) @@ -126,25 +150,6 @@ func (cc *ClientConn) process(pkt mt.Pkt) { return } - playersMu.Lock() - _, ok := players[cc.Name()] - if ok { - cc.Log("<-", "already connected") - ack, _ := cc.SendCmd(&mt.ToCltKick{Reason: mt.AlreadyConnected}) - - select { - case <-cc.Closed(): - case <-ack: - cc.Close() - } - - playersMu.Unlock() - return - } - - players[cc.Name()] = struct{}{} - playersMu.Unlock() - // reply if DefaultAuth().Exists(cc.Name()) { cc.auth.method = mt.SRP |