Open jeroenhd opened 2 years ago
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: https://github.com/matrix-org/node-irc/security/advisories/GHSA-52rh-5rpj-c3w6 which led to https://github.com/matrix-org/matrix-appservice-irc/security/advisories/GHSA-37hr-348p-rmf4). The issue is that singular \r characters are not considered line ending characters for Client.prototype.action (https://github.com/martynsmith/node-irc/blob/e4000b7a8ac42d9eb16fb6c3f362e1425d664f4b/lib/irc.js#L1017) 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: https://github.com/matrix-org/node-irc/pull/88/commits/e3eb9c15f8240e9c92365f5ffc3944469229771b 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.
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: https://github.com/matrix-org/node-irc/security/advisories/GHSA-52rh-5rpj-c3w6 which led to https://github.com/matrix-org/matrix-appservice-irc/security/advisories/GHSA-37hr-348p-rmf4). The issue is that singular \r characters are not considered line ending characters for Client.prototype.action (https://github.com/martynsmith/node-irc/blob/e4000b7a8ac42d9eb16fb6c3f362e1425d664f4b/lib/irc.js#L1017) 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: https://github.com/matrix-org/node-irc/pull/88/commits/e3eb9c15f8240e9c92365f5ffc3944469229771b 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.