From a6164cd9c40e6f5ac62d5d43a1366fe0f43efb63 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 24 May 2019 16:59:42 +0100 Subject: [PATCH 1/2] Check restricted nicknames against skeletons Fixes #519 --- irc/nickname.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/irc/nickname.go b/irc/nickname.go index a1919e59..093d8891 100644 --- a/irc/nickname.go +++ b/irc/nickname.go @@ -17,7 +17,7 @@ import ( ) var ( - // anything added here MUST be casefolded: + // anything added here MUST be skeleton'd: restrictedNicknames = map[string]bool{ "=scene=": true, // used for rp commands "histserv": true, // TODO(slingamn) this should become a real service @@ -27,7 +27,7 @@ var ( // returns whether the change succeeded or failed func performNickChange(server *Server, client *Client, target *Client, session *Session, newnick string, rb *ResponseBuffer) bool { nickname := strings.TrimSpace(newnick) - cfnick, err := CasefoldName(nickname) + skeleton, err := Skeleton(nickname) currentNick := client.Nick() if len(nickname) < 1 { @@ -35,7 +35,7 @@ func performNickChange(server *Server, client *Client, target *Client, session * return false } - if err != nil || len(nickname) > server.Config().Limits.NickLen || restrictedNicknames[cfnick] { + if err != nil || len(nickname) > server.Config().Limits.NickLen || restrictedNicknames[skeleton] { rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, nickname, client.t("Erroneous nickname")) return false } @@ -68,7 +68,7 @@ func performNickChange(server *Server, client *Client, target *Client, session * } histItem.Params[0] = nickname - client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s [%s]", origNickMask, nickname, cfnick)) + client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s [%s]", origNickMask, nickname, skeleton)) if hadNick { if client == target { target.server.snomasks.Send(sno.LocalNicks, fmt.Sprintf(ircfmt.Unescape("$%s$r changed nickname to %s"), details.nick, nickname)) From 8794740f898ecdb21061fcfbe623a38dfc4cee83 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Fri, 24 May 2019 13:09:56 -0400 Subject: [PATCH 2/2] be more pedantic about distinguishing skeletons and casefolds --- irc/accounts.go | 2 +- irc/client_lookup_set.go | 11 +++++++++-- irc/errors.go | 1 + irc/nickname.go | 26 ++++++++++++-------------- irc/services.go | 15 ++++++++++++++- 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index d60ac246..c0a1cf64 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -293,7 +293,7 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames return errAccountCreation } - if restrictedNicknames[casefoldedAccount] || restrictedNicknames[skeleton] { + if restrictedCasefoldedNicks[casefoldedAccount] || restrictedSkeletons[skeleton] { return errAccountAlreadyRegistered } diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index 007d9526..60cc4f67 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -129,13 +129,20 @@ func (clients *ClientManager) Resume(oldClient *Client, session *Session) (err e // SetNick sets a client's nickname, validating it against nicknames in use func (clients *ClientManager) SetNick(client *Client, session *Session, newNick string) error { + if len(newNick) > client.server.Config().Limits.NickLen { + return errNicknameInvalid + } newcfnick, err := CasefoldName(newNick) if err != nil { - return err + return errNicknameInvalid } newSkeleton, err := Skeleton(newNick) if err != nil { - return err + return errNicknameInvalid + } + + if restrictedCasefoldedNicks[newcfnick] || restrictedSkeletons[newSkeleton] { + return errNicknameInvalid } reservedAccount, method := client.server.accounts.EnforcementStatus(newcfnick, newSkeleton) diff --git a/irc/errors.go b/irc/errors.go index 6cbe55bf..5874f690 100644 --- a/irc/errors.go +++ b/irc/errors.go @@ -35,6 +35,7 @@ var ( errInvalidChannelName = errors.New(`Invalid channel name`) errMonitorLimitExceeded = errors.New("Monitor limit exceeded") errNickMissing = errors.New("nick missing") + errNicknameInvalid = errors.New("invalid nickname") errNicknameInUse = errors.New("nickname in use") errNicknameReserved = errors.New("nickname is reserved") errNoExistingBan = errors.New("Ban does not exist") diff --git a/irc/nickname.go b/irc/nickname.go index 093d8891..5fc3952d 100644 --- a/irc/nickname.go +++ b/irc/nickname.go @@ -17,17 +17,18 @@ import ( ) var ( - // anything added here MUST be skeleton'd: - restrictedNicknames = map[string]bool{ - "=scene=": true, // used for rp commands - "histserv": true, // TODO(slingamn) this should become a real service + restrictedNicknames = []string{ + "=scene=", // used for rp commands + "HistServ", // used to play back JOIN, PART, etc. to legacy clients } + + restrictedCasefoldedNicks = make(map[string]bool) + restrictedSkeletons = make(map[string]bool) ) // returns whether the change succeeded or failed func performNickChange(server *Server, client *Client, target *Client, session *Session, newnick string, rb *ResponseBuffer) bool { nickname := strings.TrimSpace(newnick) - skeleton, err := Skeleton(nickname) currentNick := client.Nick() if len(nickname) < 1 { @@ -35,11 +36,6 @@ func performNickChange(server *Server, client *Client, target *Client, session * return false } - if err != nil || len(nickname) > server.Config().Limits.NickLen || restrictedNicknames[skeleton] { - rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, nickname, client.t("Erroneous nickname")) - return false - } - if target.Nick() == nickname { return true } @@ -47,15 +43,17 @@ func performNickChange(server *Server, client *Client, target *Client, session * hadNick := target.HasNick() origNickMask := target.NickMaskString() details := target.Details() - err = client.server.clients.SetNick(target, session, nickname) + err := client.server.clients.SetNick(target, session, nickname) if err == errNicknameInUse { rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is already in use")) - return false } else if err == errNicknameReserved { rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is reserved by a different account")) - return false + } else if err == errNicknameInvalid { + rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, nickname, client.t("Erroneous nickname")) } else if err != nil { rb.Add(nil, server.name, ERR_UNKNOWNERROR, currentNick, "NICK", fmt.Sprintf(client.t("Could not set or change nickname: %s"), err.Error())) + } + if err != nil { return false } @@ -68,7 +66,7 @@ func performNickChange(server *Server, client *Client, target *Client, session * } histItem.Params[0] = nickname - client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s [%s]", origNickMask, nickname, skeleton)) + client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s [%s]", origNickMask, nickname, client.NickCasefolded())) if hadNick { if client == target { target.server.snomasks.Send(sno.LocalNicks, fmt.Sprintf(ircfmt.Unescape("$%s$r changed nickname to %s"), details.nick, nickname)) diff --git a/irc/services.go b/irc/services.go index 8bde6ac4..5e8026df 100644 --- a/irc/services.go +++ b/irc/services.go @@ -260,7 +260,7 @@ func initializeServices() { service.Commands["help"] = &servHelpCmd // reserve the nickname - restrictedNicknames[serviceName] = true + restrictedNicknames = append(restrictedNicknames, service.Name) // register the protocol-level commands (NICKSERV, NS) that talk to the service var ircCmdDef Command @@ -279,4 +279,17 @@ func initializeServices() { } } } + + for _, restrictedNickname := range restrictedNicknames { + cfName, err := CasefoldName(restrictedNickname) + if err != nil { + panic(err) + } + restrictedCasefoldedNicks[cfName] = true + skeleton, err := Skeleton(restrictedNickname) + if err != nil { + panic(err) + } + restrictedSkeletons[skeleton] = true + } }