Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

input should be sanitized to avoid CR-LF injection #127

Open
semarie opened this issue Aug 3, 2019 · 1 comment
Open

input should be sanitized to avoid CR-LF injection #127

semarie opened this issue Aug 3, 2019 · 1 comment

Comments

@semarie
Copy link

semarie commented Aug 3, 2019

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.

@thoj
Copy link
Owner

thoj commented Aug 7, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants