3 ms·

You need to split on (or sanitize) null bytes here: https://github.com/susam/nimb/blob/b40d87ddf2e1134fed4d7380b8ab697f6d1523d9/nimb.py#L152 https://github.com/

You need to split on (or sanitize) null bytes here: https://github.com/susam/nimb/blob/b40d87ddf2e1134fed4d7380b8ab697f6d1523d9/nimb.py#L152 https://github.com/susam/nimb/blob/b40d87ddf2e1134fed4d7380b... and sanitize NUL/CR/LF from the prefix (well, display_name) here: https://github.com/susam/nimb/blob/b40d87ddf2e1134fed4d7380b8ab697f6d1523d9/nimb.py#L155 https://github.com/susam/nimb/blob/b40d87ddf2e1134fed4d7380b... Otherwise you may have a command injection vulnerability similar to what matrix-appservice-irc used to have: https://pktz.fr/matrix/security/2022-appservice-irc-command-injection/ https://pktz.fr/matrix/security/2022-appservice-irc-command-... (though with carriage returns instead of null bytes)

susam 3y ago
Thank you for taking a close look at the source code and highlighting this issue. > You need to split on (or sanitize) null bytes Do you mean splitting on (or sanitising of) carriage returns (CR or 0xD) and newlines (LF or 0xA)? I read your security advisory and as far as I understand, the issue occurs due to improper handling of CR and LF characters. I don't see anything about null bytes there.
- progval 3y ago
  Null bytes are not allowed in IRC messages: https://modern.ircdocs.horse/#parameters https://modern.ircdocs.horse/#parameters Some IRC servers (especially those written in C) may interpret it as a line end, so you would have a similar issue
  - susam 3y ago
    Your security advisory has been quite helpful and I could reproduce the issue involving CR. It is particularly enlightening to know that the null byte could also cause the same issue because some IRC servers may interpret the null byte too as a line ending. It is late here but I have pushed a quick fix to plug this loophole: <https://github.com/susam/nimb/commit/1c02137 https://github.com/susam/nimb/commit/1c02137>. Review comments, feedback, pull requests, patches, etc. are very welcome! Thank you for this comment thread which has been constructive as well as productive.

susam 3y ago

Thank you for taking a close look at the source code and highlighting this issue. > You need to split on (or sanitize) null bytes Do you mean splitting on (or sanitising of) carriage returns (CR or 0xD) and newlines (LF or 0xA)? I read your security advisory and as far as I understand, the issue occurs due to improper handling of CR and LF characters. I don't see anything about null bytes there.

progval 3y ago
Null bytes are not allowed in IRC messages: https://modern.ircdocs.horse/#parameters https://modern.ircdocs.horse/#parameters Some IRC servers (especially those written in C) may interpret it as a line end, so you would have a similar issue
- susam 3y ago
  Your security advisory has been quite helpful and I could reproduce the issue involving CR. It is particularly enlightening to know that the null byte could also cause the same issue because some IRC servers may interpret the null byte too as a line ending. It is late here but I have pushed a quick fix to plug this loophole: <https://github.com/susam/nimb/commit/1c02137 https://github.com/susam/nimb/commit/1c02137>. Review comments, feedback, pull requests, patches, etc. are very welcome! Thank you for this comment thread which has been constructive as well as productive.