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

NUL (\x00) is not filtered from IRC args/text #2566

Open
dgw opened this issue Nov 22, 2023 · 1 comment · May be fixed by #2620
Open

NUL (\x00) is not filtered from IRC args/text #2566

dgw opened this issue Nov 22, 2023 · 1 comment · May be fixed by #2620
Labels
Bug Things to squish; generally used for issues
Milestone

Comments

@dgw
Copy link
Member

dgw commented Nov 22, 2023

Requested Feature

irc.utils.safe() strips CR and LF from its input, but not NUL (\x00) even though all three octets are disallowed in IRC lines.

sopel/sopel/irc/utils.py

Lines 20 to 48 in 3ff58c1

def safe(string):
"""Remove newlines from a string.
:param str string: input text to process
:return: the string without newlines
:rtype: str
:raises TypeError: when ``string`` is ``None``
This function removes newlines from a string and always returns a unicode
string (``str``), but doesn't strip or alter it in any other way::
>>> safe('some text\\r\\n')
'some text'
This is useful to ensure a string can be used in a IRC message.
.. versionchanged:: 7.1
This function now raises a :exc:`TypeError` instead of an unpredictable
behaviour when given ``None``.
"""
if string is None:
raise TypeError('safe function requires a string, not NoneType')
if isinstance(string, bytes):
string = string.decode("utf8")
string = string.replace('\n', '')
string = string.replace('\r', '')
return string

Problems Solved

Plugins accidentally sending NUL will most likely cause the server to disconnect the bot with an error ("malformed data" or similar).

Alternatives

Plugin authors must manually ensure that anything their code sends to IRC does not contain the NUL byte.

Notes

No response

@dgw dgw added Feature Needs Triage Issues that need to be reviewed and categorized labels Nov 22, 2023
@dgw
Copy link
Member Author

dgw commented Sep 11, 2024

Remembered this and finally tested it:

@plugin.command('nullme')
@plugin.require_admin("Sorry, I can't let you do that.", reply=True)
@plugin.output_prefix('[NULL] ')
def null(bot, trigger):
    """Send a message containing null, for testing."""
    bot.say("Null\x00byte!")

At least on Rizon, the bot is not disconnected; the message gets truncated:

<dgw> .nullme
<SopelGitpod> [NULL] Null

Obviously not ideal, but not as bad as it could be.

@dgw dgw linked a pull request Sep 14, 2024 that will close this issue
4 tasks
@dgw dgw added Bug Things to squish; generally used for issues and removed Feature Needs Triage Issues that need to be reviewed and categorized labels Sep 14, 2024
@dgw dgw added this to the 8.1.0 milestone Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant