3
0
mirror of https://github.com/ergochat/ergo.git synced 2024-11-10 22:19:31 +01:00

NICK: Prevent races, remove a DoS

This commit is contained in:
Daniel Oaks 2016-11-16 12:02:22 +10:00
parent 95e36b99a2
commit 9a9820fa88
5 changed files with 54 additions and 54 deletions

View File

@ -19,6 +19,8 @@ New release of Oragono!
### Removed
### Fixed
* Prevented a DoS related to lots of clients connecting at once.
* Removed races around setting and changing `NICK`s, to be more safe.
* Fixed crash when using STATUSMSG-like messaging.
* Fixed crash with gIRC-Go ircmsg library we depend on.

View File

@ -14,8 +14,6 @@ import (
"strconv"
"time"
"strings"
"github.com/DanielOaks/girc-go/ircmsg"
"github.com/DanielOaks/go-ident"
)
@ -146,14 +144,12 @@ func (client *Client) run() {
client.rawHostname = AddrLookupHostname(client.socket.conn.RemoteAddr())
//TODO(dan): Make this a socketreactor from ircbnc
fmt.Println("START", &client)
for {
line, err = client.socket.Read()
if err != nil {
client.Quit("connection closed")
break
}
fmt.Println(" LINE", &client, strings.TrimSpace(line))
msg, err = ircmsg.ParseLine(line)
if err != nil {
@ -163,7 +159,6 @@ func (client *Client) run() {
cmd, exists := Commands[msg.Command]
if !exists {
fmt.Println(" BADLINE", &client, strings.TrimSpace(line))
if len(msg.Command) > 0 {
client.Send(nil, client.server.name, ERR_UNKNOWNCOMMAND, client.nick, msg.Command, "Unknown command")
} else {
@ -171,15 +166,11 @@ func (client *Client) run() {
}
continue
}
fmt.Println(" GUDLINE", &client, strings.TrimSpace(line))
isExiting = cmd.Run(client.server, client, msg)
fmt.Println(" CMDRUN", &client, strings.TrimSpace(line))
if isExiting || client.isQuitting {
fmt.Println(" BREAKING", &client, strings.TrimSpace(line))
break
}
fmt.Println(" CONTINUE", &client, strings.TrimSpace(line))
}
// ensure client connection gets closed
@ -368,22 +359,22 @@ func (client *Client) SetNickname(nickname string) error {
client.nick = nickname
client.updateNick()
client.server.clients.Add(client)
return nil
return client.server.clients.Add(client)
}
// ChangeNickname changes the existing nickname of the client.
func (client *Client) ChangeNickname(nickname string) error {
origNickMask := client.nickMaskString
client.server.clients.Remove(client)
client.server.whoWas.Append(client)
client.nick = nickname
client.updateNickMask()
client.server.clients.Add(client)
for friend := range client.Friends() {
friend.Send(nil, origNickMask, "NICK", nickname)
err := client.server.clients.Replace(client.nick, nickname, client)
if err == nil {
client.server.whoWas.Append(client)
client.nick = nickname
for friend := range client.Friends() {
friend.Send(nil, origNickMask, "NICK", nickname)
}
}
return nil
return err
}
func (client *Client) Quit(message string) {

View File

@ -13,8 +13,6 @@ import (
"sync"
"runtime/debug"
"github.com/DanielOaks/girc-go/ircmatch"
)
@ -37,24 +35,8 @@ func ExpandUserHost(userhost string) (expanded string) {
return
}
type LoggingMutex struct {
mutex sync.Mutex
}
func (lm *LoggingMutex) Lock() {
lm.mutex.Lock()
fmt.Println("--- locked, stack:")
debug.PrintStack()
}
func (lm *LoggingMutex) Unlock() {
fmt.Println("--- unlocking")
lm.mutex.Unlock()
fmt.Println("--- unlocked")
}
type ClientLookupSet struct {
ByNickMutex LoggingMutex
ByNickMutex sync.Mutex
ByNick map[string]*Client
}
@ -83,6 +65,7 @@ func (clients *ClientLookupSet) Has(nick string) bool {
return exists
}
// getNoMutex is used internally, for getting clients when no mutex is required (i.e. is already set).
func (clients *ClientLookupSet) getNoMutex(nick string) *Client {
casefoldedName, err := CasefoldName(nick)
if err == nil {
@ -104,23 +87,15 @@ func (clients *ClientLookupSet) Get(nick string) *Client {
}
func (clients *ClientLookupSet) Add(client *Client) error {
fmt.Println("Initial nick:", client.nick)
if !client.HasNick() {
return ErrNickMissing
}
fmt.Println("- getting lock:", client.nick)
clients.ByNickMutex.Lock()
fmt.Println("- got lock:", client.nick)
defer clients.ByNickMutex.Unlock()
if clients.getNoMutex(client.nick) != nil {
fmt.Println("- already exists:", client.nick)
clients.ByNickMutex.Unlock()
return ErrNicknameInUse
}
fmt.Println("- adding:", client.nick)
clients.ByNick[client.nickCasefolded] = client
fmt.Println("- set:", client.nick)
clients.ByNickMutex.Unlock()
fmt.Println("- released lock:", client.nick)
return nil
}
@ -128,15 +103,49 @@ func (clients *ClientLookupSet) Remove(client *Client) error {
if !client.HasNick() {
return ErrNickMissing
}
clients.ByNickMutex.Lock()
if clients.getNoMutex(client.nick) != client {
clients.ByNickMutex.Unlock()
return ErrNicknameMismatch
}
clients.ByNickMutex.Lock()
delete(clients.ByNick, client.nickCasefolded)
clients.ByNickMutex.Unlock()
return nil
}
func (clients *ClientLookupSet) Replace(oldNick, newNick string, client *Client) error {
// get casefolded nicknames
oldNick, err := CasefoldName(oldNick)
if err != nil {
return err
}
newNick, err = CasefoldName(newNick)
if err != nil {
return err
}
// remove and replace
clients.ByNickMutex.Lock()
defer clients.ByNickMutex.Unlock()
oldClient := clients.getNoMutex(newNick)
if oldClient != nil {
return ErrNicknameInUse
}
if oldClient != client {
return ErrNicknameMismatch
}
if oldNick == newNick {
// if they're only changing case, don't need to remove+re-add them
return nil
}
delete(clients.ByNick, oldNick)
clients.ByNick[newNick] = client
return nil
}
func (clients *ClientLookupSet) AllWithCaps(caps ...Capability) (set ClientSet) {
set = make(ClientSet)

View File

@ -5,6 +5,7 @@
package irc
import (
"fmt"
"strings"
"github.com/DanielOaks/girc-go/ircmsg"
@ -35,19 +36,17 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
}
// bleh, this will be replaced and done below
target := server.clients.Get(nickname)
if target != nil && target != client {
client.Send(nil, server.name, ERR_NICKNAMEINUSE, client.nick, nicknameRaw, "Nickname is already in use")
return false
}
if client.registered {
err = client.ChangeNickname(nicknameRaw)
} else {
err = client.SetNickname(nicknameRaw)
}
if err != nil {
if err == ErrNicknameInUse {
client.Send(nil, server.name, ERR_NICKNAMEINUSE, client.nick, nicknameRaw, "Nickname is already in use")
return false
} else if err != nil {
client.Send(nil, server.name, ERR_UNKNOWNERROR, client.nick, "NICK", fmt.Sprintf("Could not set or change nickname: %s", err.Error()))
return false
}
if client.registered {
client.alertMonitors()

View File

@ -567,7 +567,6 @@ func (server *Server) wslisten(addr string, tlsMap map[string]*TLSListenConfig)
func (server *Server) tryRegister(c *Client) {
if c.registered || !c.HasNick() || !c.HasUsername() ||
(c.capState == CapNegotiating) {
fmt.Println("Try Reg:", &c, c.registered, c.HasNick(), c.HasUsername(), c.capState == CapNegotiating, c.capState)
return
}
c.Register()