diff --git a/default.yaml b/default.yaml index d101396b..bb785fb8 100644 --- a/default.yaml +++ b/default.yaml @@ -235,6 +235,13 @@ server: # this works around that bug, allowing them to use SASL. send-unprefixed-sasl: true + # traditionally, IRC servers will truncate and send messages that are + # too long to be relayed intact. this behavior can be disabled by setting + # allow-truncation to false, in which case Oragono will reject the message + # and return an error to the client. (note that this option defaults to true + # when unset.) + allow-truncation: false + # IP-based DoS protection ip-limits: # whether to limit the total number of concurrent connections per IP/CIDR diff --git a/go.mod b/go.mod index cdfadc7e..e9963c92 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/go-sql-driver/mysql v1.5.0 github.com/go-test/deep v1.0.6 // indirect github.com/gorilla/websocket v1.4.2 - github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847 + github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96 github.com/onsi/ginkgo v1.12.0 // indirect github.com/onsi/gomega v1.9.0 // indirect github.com/oragono/confusables v0.0.0-20201108231250-4ab98ab61fb1 @@ -24,4 +24,4 @@ require ( gopkg.in/yaml.v2 v2.3.0 ) -replace github.com/gorilla/websocket => github.com/oragono/websocket v1.4.2-oragono1 +replace github.com/gorilla/websocket => github.com/oragono/websocket v1.4.2-oragono1 diff --git a/go.sum b/go.sum index 402f78e9..d4284920 100644 --- a/go.sum +++ b/go.sum @@ -40,6 +40,8 @@ github.com/goshuirc/irc-go v0.0.0-20210223005429-8d38e43fc6ed h1:cwwqHrmLafgEucS github.com/goshuirc/irc-go v0.0.0-20210223005429-8d38e43fc6ed/go.mod h1:q/JhvvKLmif3y9q8MDQM+gRCnjEKnu5ClF298TTXJug= github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847 h1:MmsZRpAsMxyw0P5/SFn2L6edhmIXRlolgXvOF+fgEiQ= github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847/go.mod h1:q/JhvvKLmif3y9q8MDQM+gRCnjEKnu5ClF298TTXJug= +github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96 h1:sihI3HsrJWyS4MtBmxh5W4gDZD34SWodkWyUvJltswY= +github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96/go.mod h1:q/JhvvKLmif3y9q8MDQM+gRCnjEKnu5ClF298TTXJug= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= diff --git a/irc/channel.go b/irc/channel.go index e9e47c78..09a8a956 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -1361,6 +1361,15 @@ func (channel *Channel) SendSplitMessage(command string, minPrefixMode modes.Mod details := client.Details() chname := channel.Name() + if !client.server.Config().Server.Compatibility.allowTruncation { + if !validateSplitMessageLen(histType, details.nickMask, chname, message) { + rb.Add(nil, client.server.name, ERR_INPUTTOOLONG, details.nick, client.t("Line too long to be relayed without truncation")) + // TODO(#1577) remove this logline: + client.server.logger.Debug("internal", "rejected truncation-requiring DM from client", details.nick) + return + } + } + // STATUSMSG targets are prefixed with the supplied min-prefix, e.g., @#channel if minPrefixMode != modes.Mode(0) { chname = fmt.Sprintf("%s%s", modes.ChannelModePrefixes[minPrefixMode], chname) diff --git a/irc/client.go b/irc/client.go index 7a03d0f1..8bffd5fd 100644 --- a/irc/client.go +++ b/irc/client.go @@ -740,8 +740,13 @@ func (client *Client) run(session *Session) { msg, err := ircmsg.ParseLineStrict(line, true, MaxLineLen) if err == ircmsg.ErrorLineIsEmpty { continue - } else if err == ircmsg.ErrorLineTooLong { + } else if err == ircmsg.ErrorTagsTooLong { + session.Send(nil, client.server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Input line contained excess tag data")) + continue + } else if err == ircmsg.ErrorBodyTooLong && !client.server.Config().Server.Compatibility.allowTruncation { session.Send(nil, client.server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Input line too long")) + // TODO(#1577) remove this logline: + client.server.logger.Debug("internal", "rejected MaxLineLen-exceeding line from client", client.Nick()) continue } else if err != nil { client.Quit(client.t("Received malformed line"), session) @@ -1711,7 +1716,7 @@ func (session *Session) SendRawMessage(message ircmsg.IRCMessage, blocking bool) // assemble message line, err := message.LineBytesStrict(false, MaxLineLen) - if err != nil { + if !(err == nil || err == ircmsg.ErrorBodyTooLong) { errorParams := []string{"Error assembling message for sending", err.Error(), message.Command} errorParams = append(errorParams, message.Params...) session.client.server.logger.Error("internal", errorParams...) diff --git a/irc/config.go b/irc/config.go index a050b46a..c0a831d1 100644 --- a/irc/config.go +++ b/irc/config.go @@ -569,7 +569,9 @@ type Config struct { Compatibility struct { ForceTrailing *bool `yaml:"force-trailing"` forceTrailing bool - SendUnprefixedSasl bool `yaml:"send-unprefixed-sasl"` + SendUnprefixedSasl bool `yaml:"send-unprefixed-sasl"` + AllowTruncation *bool `yaml:"allow-truncation"` + allowTruncation bool } isupport isupport.List IPLimits connection_limits.LimiterConfig `yaml:"ip-limits"` @@ -1378,6 +1380,7 @@ func LoadConfig(filename string) (config *Config, err error) { } config.Server.Compatibility.forceTrailing = utils.BoolDefaultTrue(config.Server.Compatibility.ForceTrailing) + config.Server.Compatibility.allowTruncation = utils.BoolDefaultTrue(config.Server.Compatibility.AllowTruncation) config.loadMOTD() diff --git a/irc/handlers.go b/irc/handlers.go index 1598b698..1938830f 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -1981,6 +1981,43 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IRCMessage, rb *Resp return false } +// check whether a PRIVMSG or NOTICE is too long to be relayed without truncation +func validateLineLen(msgType history.ItemType, source, target, payload string) (ok bool) { + // :source PRIVMSG #target :payload\r\n + // 1: initial colon on prefix + // 1: space between prefix and command + // 1: space between command and target (first parameter) + // 1: space between target and payload (second parameter) + // 1: colon to send the payload as a trailing (we force trailing for PRIVMSG and NOTICE) + // 2: final \r\n + limit := MaxLineLen - 7 + limit -= len(source) + switch msgType { + case history.Privmsg: + limit -= 7 + case history.Notice: + limit -= 6 + default: + return true + } + limit -= len(payload) + return limit >= 0 +} + +// check validateLineLen for an entire SplitMessage (which may consist of multiple lines) +func validateSplitMessageLen(msgType history.ItemType, source, target string, message utils.SplitMessage) (ok bool) { + if message.Is512() { + return validateLineLen(msgType, source, target, message.Message) + } else { + for _, messagePair := range message.Split { + if !validateLineLen(msgType, source, target, messagePair.Message) { + return false + } + } + return true + } +} + // helper to store a batched PRIVMSG in the session object func absorbBatchedMessage(server *Server, client *Client, msg ircmsg.IRCMessage, batchTag string, histType history.ItemType, rb *ResponseBuffer) { var errorCode, errorMessage string @@ -2033,7 +2070,6 @@ func messageHandler(server *Server, client *Client, msg ircmsg.IRCMessage, rb *R return false } - cnick := client.Nick() clientOnlyTags := msg.ClientOnlyTags() if histType == history.Tagmsg && len(clientOnlyTags) == 0 { // nothing to do @@ -2046,7 +2082,7 @@ func messageHandler(server *Server, client *Client, msg ircmsg.IRCMessage, rb *R message = msg.Params[1] } if histType != history.Tagmsg && message == "" { - rb.Add(nil, server.name, ERR_NOTEXTTOSEND, cnick, client.t("No text to send")) + rb.Add(nil, server.name, ERR_NOTEXTTOSEND, client.Nick(), client.t("No text to send")) return false } @@ -2147,6 +2183,14 @@ func dispatchMessageToTarget(client *Client, tags map[string]string, histType hi rb.Add(nil, server.name, ERR_NEEDREGGEDNICK, client.Nick(), tnick, client.t("You must be registered to send a direct message to this user")) return } + if !client.server.Config().Server.Compatibility.allowTruncation { + if !validateSplitMessageLen(histType, client.NickMaskString(), tnick, message) { + rb.Add(nil, server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Line too long to be relayed without truncation")) + // TODO(#1577) remove this logline: + client.server.logger.Debug("internal", "rejected truncation-requiring channel message from client", details.nick) + return + } + } nickMaskString := details.nickMask accountName := details.accountName var deliverySessions []*Session diff --git a/irc/message_cache.go b/irc/message_cache.go index e80792e4..4f4c20d9 100644 --- a/irc/message_cache.go +++ b/irc/message_cache.go @@ -54,7 +54,7 @@ func addAllTags(msg *ircmsg.IRCMessage, tags map[string]string, serverTime time. } func (m *MessageCache) handleErr(server *Server, err error) bool { - if err != nil { + if !(err == nil || err == ircmsg.ErrorBodyTooLong) { server.logger.Error("internal", "Error assembling message for sending", err.Error()) // blank these out so Send will be a no-op m.fullTags = nil diff --git a/traditional.yaml b/traditional.yaml index ba9141df..3dc24556 100644 --- a/traditional.yaml +++ b/traditional.yaml @@ -208,6 +208,13 @@ server: # this works around that bug, allowing them to use SASL. send-unprefixed-sasl: true + # traditionally, IRC servers will truncate and send messages that are + # too long to be relayed intact. this behavior can be disabled by setting + # allow-truncation to false, in which case Oragono will reject the message + # and return an error to the client. (note that this option defaults to true + # when unset.) + allow-truncation: true + # IP-based DoS protection ip-limits: # whether to limit the total number of concurrent connections per IP/CIDR diff --git a/vendor/github.com/goshuirc/irc-go/ircmsg/message.go b/vendor/github.com/goshuirc/irc-go/ircmsg/message.go index 2442d074..a1d8f4b3 100644 --- a/vendor/github.com/goshuirc/irc-go/ircmsg/message.go +++ b/vendor/github.com/goshuirc/irc-go/ircmsg/message.go @@ -9,6 +9,7 @@ import ( "bytes" "errors" "strings" + "unicode/utf8" ) const ( @@ -34,17 +35,30 @@ const ( var ( // ErrorLineIsEmpty indicates that the given IRC line was empty. ErrorLineIsEmpty = errors.New("Line is empty") + // ErrorLineContainsBadChar indicates that the line contained invalid characters ErrorLineContainsBadChar = errors.New("Line contains invalid characters") - // ErrorLineTooLong indicates that the message exceeded the maximum tag length - // (the name references 417 ERR_INPUTTOOLONG; we reserve the right to return it - // for messages that exceed the non-tag length limit) - ErrorLineTooLong = errors.New("Line could not be parsed because a specified length limit was exceeded") + + // ErrorBodyTooLong indicates that the message body exceeded the specified + // length limit (typically 512 bytes). This error is non-fatal; if encountered + // when parsing a message, the message is parsed up to the length limit, and + // if encountered when serializing a message, the message is truncated to the limit. + ErrorBodyTooLong = errors.New("Line body exceeded the specified length limit; outgoing messages will be truncated") + + // ErrorTagsTooLong indicates that the message exceeded the maximum tag length + // (the specified response on the server side is 417 ERR_INPUTTOOLONG). + ErrorTagsTooLong = errors.New("Line could not be processed because its tag data exceeded the length limit") + // ErrorInvalidTagContent indicates that a tag name or value was invalid ErrorInvalidTagContent = errors.New("Line could not be processed because it contained an invalid tag name or value") + // ErrorCommandMissing indicates that an IRC message was invalid because it lacked a command. ErrorCommandMissing = errors.New("IRC messages MUST have a command") - ErrorBadParam = errors.New("Cannot have an empty param, a param with spaces, or a param that starts with ':' before the last parameter") + + // ErrorBadParam indicates that an IRC message could not be serialized because + // its parameters violated the syntactic constraints on IRC parameters: + // non-final parameters cannot be empty, contain a space, or start with `:`. + ErrorBadParam = errors.New("Cannot have an empty param, a param with spaces, or a param that starts with ':' before the last parameter") ) // IRCMessage represents an IRC message, as defined by the RFCs and as @@ -161,7 +175,7 @@ func ParseLineStrict(line string, fromClient bool, truncateLen int) (ircmsg IRCM // slice off any amount of ' ' from the front of the string func trimInitialSpaces(str string) string { var i int - for i = 0; i < len(str) && str[i] == ' '; i += 1 { + for i = 0; i < len(str) && str[i] == ' '; i++ { } return str[i:] } @@ -170,6 +184,14 @@ func parseLine(line string, maxTagDataLength int, truncateLen int) (ircmsg IRCMe // remove either \n or \r\n from the end of the line: line = strings.TrimSuffix(line, "\n") line = strings.TrimSuffix(line, "\r") + // whether we removed them ourselves, or whether they were removed previously, + // they count against the line limit: + if truncateLen != 0 { + if truncateLen <= 2 { + return ircmsg, ErrorLineIsEmpty + } + truncateLen -= 2 + } // now validate for the 3 forbidden bytes: if strings.IndexByte(line, '\x00') != -1 || strings.IndexByte(line, '\n') != -1 || strings.IndexByte(line, '\r') != -1 { return ircmsg, ErrorLineContainsBadChar @@ -187,7 +209,7 @@ func parseLine(line string, maxTagDataLength int, truncateLen int) (ircmsg IRCMe } tags := line[1:tagEnd] if 0 < maxTagDataLength && maxTagDataLength < len(tags) { - return ircmsg, ErrorLineTooLong + return ircmsg, ErrorTagsTooLong } err = ircmsg.parseTags(tags) if err != nil { @@ -198,7 +220,8 @@ func parseLine(line string, maxTagDataLength int, truncateLen int) (ircmsg IRCMe } // truncate if desired - if 0 < truncateLen && truncateLen < len(line) { + if truncateLen != 0 && truncateLen < len(line) { + err = ErrorBodyTooLong line = line[:truncateLen] } @@ -252,7 +275,7 @@ func parseLine(line string, maxTagDataLength int, truncateLen int) (ircmsg IRCMe line = line[paramEnd+1:] } - return ircmsg, nil + return ircmsg, err } // helper to parse tags @@ -337,8 +360,8 @@ func paramRequiresTrailing(param string) bool { } // line returns a sendable line created from an IRCMessage. -func (ircmsg *IRCMessage) line(tagLimit, clientOnlyTagDataLimit, serverAddedTagDataLimit, truncateLen int) ([]byte, error) { - if len(ircmsg.Command) < 1 { +func (ircmsg *IRCMessage) line(tagLimit, clientOnlyTagDataLimit, serverAddedTagDataLimit, truncateLen int) (result []byte, err error) { + if len(ircmsg.Command) == 0 { return nil, ErrorCommandMissing } @@ -382,10 +405,10 @@ func (ircmsg *IRCMessage) line(tagLimit, clientOnlyTagDataLimit, serverAddedTagD lenTags = buf.Len() if 0 < tagLimit && tagLimit < buf.Len() { - return nil, ErrorLineTooLong + return nil, ErrorTagsTooLong } if (0 < clientOnlyTagDataLimit && clientOnlyTagDataLimit < lenClientOnlyTags) || (0 < serverAddedTagDataLimit && serverAddedTagDataLimit < lenRegularTags) { - return nil, ErrorLineTooLong + return nil, ErrorTagsTooLong } if len(ircmsg.Prefix) > 0 { @@ -408,18 +431,33 @@ func (ircmsg *IRCMessage) line(tagLimit, clientOnlyTagDataLimit, serverAddedTagD buf.WriteString(param) } - // truncate if desired - // -2 for \r\n - restLen := buf.Len() - lenTags - if 0 < truncateLen && (truncateLen-2) < restLen { - buf.Truncate(lenTags + (truncateLen - 2)) + // truncate if desired; leave 2 bytes over for \r\n: + if truncateLen != 0 && (truncateLen-2) < (buf.Len()-lenTags) { + err = ErrorBodyTooLong + newBufLen := lenTags + (truncateLen - 2) + buf.Truncate(newBufLen) + // XXX: we may have truncated in the middle of a UTF8-encoded codepoint; + // if so, remove additional bytes, stopping when the sequence either + // ends in a valid codepoint, or we have removed 3 bytes (the maximum + // length of the remnant of a once-valid, truncated codepoint; we don't + // want to truncate the entire message if it wasn't UTF8 in the first + // place). + for i := 0; i < (utf8.UTFMax - 1); i++ { + r, n := utf8.DecodeLastRune(buf.Bytes()) + if r == utf8.RuneError && n <= 1 { + newBufLen-- + buf.Truncate(newBufLen) + } else { + break + } + } } buf.WriteString("\r\n") - result := buf.Bytes() + result = buf.Bytes() toValidate := result[:len(result)-2] if bytes.IndexByte(toValidate, '\x00') != -1 || bytes.IndexByte(toValidate, '\r') != -1 || bytes.IndexByte(toValidate, '\n') != -1 { return nil, ErrorLineContainsBadChar } - return result, nil + return result, err } diff --git a/vendor/github.com/goshuirc/irc-go/ircreader/ircreader.go b/vendor/github.com/goshuirc/irc-go/ircreader/ircreader.go index 4411fac0..204345d7 100644 --- a/vendor/github.com/goshuirc/irc-go/ircreader/ircreader.go +++ b/vendor/github.com/goshuirc/irc-go/ircreader/ircreader.go @@ -63,6 +63,10 @@ func (cc *IRCReader) ReadLine() ([]byte, error) { line := cc.buf[cc.start : cc.searchFrom+nlidx] cc.start = cc.searchFrom + nlidx + 1 cc.searchFrom = cc.start + // treat \r\n as the line terminator if it was present + if 0 < len(line) && line[len(line)-1] == '\r' { + line = line[:len(line)-1] + } return line, nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index c4bd08cc..68def9e2 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -21,7 +21,7 @@ github.com/go-sql-driver/mysql # github.com/gorilla/websocket v1.4.2 => github.com/oragono/websocket v1.4.2-oragono1 ## explicit github.com/gorilla/websocket -# github.com/goshuirc/irc-go v0.0.0-20210301225436-2c4b83d64847 +# github.com/goshuirc/irc-go v0.0.0-20210304031553-cf78e9176f96 ## explicit github.com/goshuirc/irc-go/ircfmt github.com/goshuirc/irc-go/ircmsg