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

Consider updating the CR/CR+LF/LF matching code for detecting lines in messages #573

Open
jeroenhd opened this issue May 4, 2022 · 0 comments

Comments

@jeroenhd
Copy link

jeroenhd commented May 4, 2022

Please note: I am not an IRC expert nor a user of this library.

The people over at Matrix.org have forked this repository (https://github.com/matrix-org/node-irc) and ran into a security vulnerability because of the way strings are split on line end characters (see: GHSA-52rh-5rpj-c3w6 which led to GHSA-37hr-348p-rmf4). The issue is that singular \r characters are not considered line ending characters for Client.prototype.action (

text.toString().split(/\r?\n/).filter(function(line) {
) while I believe they should be according to the spec. This allows unwitting users of this library processing untrusted data to allow executing IRC commands, which is exactly what happens in the Matrix vulnerability.

For all I know this is not a security issue in the actual library. I did notice, though, that CR is supposed to be a line ending character and the split regex that the Matrix fork uses is still in this library.

If this is a bug, you can probably apply the seven character fix that can be found here: matrix-org@e3eb9c1
You can also see the full details of the patched release, including some added tests, here: https://github.com/matrix-org/node-irc/pull/88/files

A direct pull request for upstreaming is going to be difficult as it seems the Matrix folks have rewritten their fork into Typescript.

If this issue is NOT considered an issue for this library, but only a problem for the downstream fork(s), feel free to close it.

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

1 participant