cinchrb / cinch

The IRC Bot Building Framework
http://www.rubydoc.info/gems/cinch
MIT License
1k stars 180 forks source link

Security fixes and tests #255

Closed rennex closed 5 years ago

rennex commented 6 years ago

I noticed a security issue with sending messages that could contain data from external sources, namely that there might be newlines that would then enable injecting IRC commands to the bot's connection. When starting to fix that, I also noticed that the existing tests were not running correctly, so I made a handful of changes to fix them.

I added tests for my fixes, and the basic case of checking what gets sent over the wire took a bit of work to implement. This is why I split off process_one from MessageQueue.process!.

Target#action doesn't split long messages (which might be a useful feature), so its way of constructing a raw IRC command made it possible to inject a newline. I added a more generic fix in MessageQueue to prevent ever sending more than one line per call. I think that this change is necessary, even if it might break some weird bot or plugin implementation.

Finally, I noticed that sanitize() was supposed to remove newlines according to its documentation, but it didn't.