go-ircevent icon indicating copy to clipboard operation
go-ircevent copied to clipboard

input should be sanitized to avoid CR-LF injection

Open semarie opened this issue 6 years ago • 1 comments

Lot of functions (Join, Part, Notice, Action, Privmsg, Kick, ... and ...f counter-parts) writes message verbatim without doing any string sanitization.

The RFC1459 stands that a message is:

IRC messages are always lines of characters terminated with a CR-LF
(Carriage Return - Line Feed) pair, and these messages shall not
exceed 512 characters in length, counting all characters including
the trailing CR-LF.

As no escape mecanism exists, it implies CR-LF should not be present in the message.

If I keep the length problem outside of the scope, your implementation allows to use CR-LF ("\r\n") in the message. It means a user could pass raw IRC commands in the message by injecting CR-LF sequence in the message.

irc_con.Privmsg("#target", "message 1\r\nKICK #target user :message 2")

If message comes from untrusted input, it could lead to security issue: the user could gain privileges or assume identity (the one of the irc_con).

I think an error should be returned if the message contains CR-LF.

semarie avatar Aug 03 '19 11:08 semarie

Pull request welcome. I suggest instead of error that we use an escape function for all input that just replaces CR-LR etc with printable strings.

thoj avatar Aug 07 '19 11:08 thoj