kiwiirc / irc-framework

🛠️ A better IRC framework for node.js. For bots and full clients.
MIT License
181 stars 63 forks source link

SASL message chunking is broken #296

Closed aaronmdjones closed 2 years ago

aaronmdjones commented 3 years ago

In the following lines:

https://github.com/kiwiirc/irc-framework/blob/a515b1bd3c34aab8f30c6e97c4abb3f0d36dd8bb/src/commands/handlers/registration.js#L271-L274

The second parameter to str.slice() is a non-inclusive range; you are only selecting the first 399 characters, not the first 400.

It should be:

        handler.connection.write('AUTHENTICATE ' + b64.slice(0, 400));
xPaw commented 3 years ago
var abc = "example";
abc.slice(0, 2); // "ex"
abc.slice(2) // "ample"

Selecting 399 characters to send instead of 400 is not a problem in itself, all characters will be sent regardless. Why is this an issue exactly?

aaronmdjones commented 3 years ago

It's an issue because you must send 400 characters at a time. The specification says that chunks less than 400 characters are the end of a message; services will then assume that's all the data there is, and a PLAIN login with a long password will fail.

xPaw commented 3 years ago

Interesting. This code dates back at least 7 years, and no one ran into this issue before?

Did your auth fail because of this? Have you tested that changing 399 to 400 correctly fixes it?

aaronmdjones commented 3 years ago

It would require a long password; probably longer than most people have tested.

Assuming a 16-char nickname (the max allowed on the popular networks, in my experience), and the fact that this codebase doesn't send a zero-length authzid (so it sends the nickname twice), you'd need a password greater than or equal to 266 characters in length:

$ printf '%s\0%s\0%s' 'Char-16-Nickname' 'Char-16-Nickname' $(i=266; while [[ $i -gt 0 ]] ; do echo -n 'a' ; i=$((i-1)) ; done) | base64 -w0 | wc -c

(400)

glguy commented 3 years ago

For reference splitting on 400 is specified in https://ircv3.net/specs/extensions/sasl-3.1#the-authenticate-command , though it's a bit academic since amdj works on the service that requires 400 splits to work.

Receiving 400 characters informs the service to expect more. Fewer and the service knows you're done sending bytes.