Add config variable supybot.commands.allowShell. (#1301)

ProgVal · Sep 24, 2017 · 4f6a5e7db9ee586120fda8138b5e3210ad54a011 · 4f6a5e7
1 parent 82cdf17
commit 4f6a5e7db9ee586120fda8138b5e3210ad54a011
diff --git a/plugins/Config/plugin.py b/plugins/Config/plugin.py
@@ -76,6 +76,29 @@ def getCapability(name):
        ### Do more later, for specific capabilities/sections.
    return capability

+def isReadOnly(name):
+    """Prevents changing certain config variables to gain shell access via
+    a vulnerable IRC network."""
+    parts = registry.split(name.lower())
+    if parts[0] != 'supybot':
+        parts.insert(0, 'supybot')
+    if parts == ['supybot', 'commands', 'allowshell'] and \
+            not conf.supybot.commands.allowShell():
+        # allow setting supybot.commands.allowShell from True to False,
+        # but not from False to True.
+        # Otherwise an IRC network could overwrite it.
+        return True
+    elif parts[0:2] == ['supybot', 'directories'] and \
+            not conf.supybot.commands.allowShell():
+        # Setting plugins directory allows for arbitrary code execution if
+        # an attacker can both use the IRC network to MITM and upload files
+        # on the server (eg. with a web CMS).
+        # Setting other directories allows writing data at arbitrary
+        # locations.
+        return True
+    else:
+        return False
+
 def _reload():
    ircdb.users.reload()
    ircdb.ignores.reload()
@@ -189,6 +212,10 @@ def _getValue(self, irc, msg, group, addChannel=False):
                      'available in this group.'))

    def _setValue(self, irc, msg, group, value):
+        if isReadOnly(group._name):
+            irc.error(_('This configuration variable is not writeable '
+                'via IRC. To change it you have to: 1) use @flush 2) edit '
+                'the config file 3) use @config reload.'), Raise=True)
        capability = getCapability(group._name)
        if ircdb.checkCapability(msg.prefix, capability):
            # I think callCommand catches exceptions here.  Should it?
@@ -294,6 +321,10 @@ def export(self, irc, msg, args, filename):
        command will export a "sanitized" configuration file suitable for
        showing publicly.
        """
+        if not conf.supybot.commands.allowShell():
+            # Disallow writing arbitrary files
+            irc.error('This command is not available, because '
+                'supybot.commands.allowShell is False.', Raise=True)
        registry.close(conf.supybot, filename, private=False)
        irc.replySuccess()
    export = wrap(export, [('checkCapability', 'owner'), 'filename'])
diff --git a/plugins/Config/test.py b/plugins/Config/test.py
@@ -80,5 +80,35 @@ def testConfigErrors(self):
                          'True or False.*, not \'123\'.')
        self.assertRegexp('config supybot.replies foo', 'settable')

+    def testReadOnly(self):
+        old_plugins_dirs = conf.supybot.directories.plugins()
+        try:
+            self.assertResponse('config supybot.commands.allowShell', 'True')
+            self.assertNotError('config supybot.directories.plugins dir1')
+            self.assertNotError('config supybot.commands.allowShell True')
+            self.assertResponse('config supybot.commands.allowShell', 'True')
+            self.assertResponse('config supybot.directories.plugins', 'dir1')
+
+            self.assertNotError('config supybot.commands.allowShell False')
+            self.assertResponse('config supybot.commands.allowShell', 'False')
+
+            self.assertRegexp('config supybot.directories.plugins dir2',
+                    'Error.*not writeable')
+            self.assertResponse('config supybot.directories.plugins', 'dir1')
+            self.assertRegexp('config supybot.commands.allowShell True',
+                    'Error.*not writeable')
+            self.assertResponse('config supybot.commands.allowShell', 'False')
+
+            self.assertRegexp('config commands.allowShell True',
+                    'Error.*not writeable')
+            self.assertResponse('config supybot.commands.allowShell', 'False')
+
+            self.assertRegexp('config COMMANDS.ALLOWSHELL True',
+                    'Error.*not writeable')
+            self.assertResponse('config supybot.commands.allowShell', 'False')
+        finally:
+            conf.supybot.commands.allowShell.setValue(True)
+            conf.supybot.directories.plugins.setValue(old_plugins_dirs)
+
 # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79:

diff --git a/plugins/Debug/plugin.py b/plugins/Debug/plugin.py
@@ -66,6 +66,12 @@ def tracer(frame, event, _):
            fd.write('%s: %s\n' % (code.co_filename, code.co_name))
    return tracer

+def checkAllowShell(irc):
+    if not conf.supybot.commands.allowShell():
+        irc.error('This command is not available, because '
+            'supybot.commands.allowShell is False.', Raise=True)
+
+
 class Debug(callbacks.Privmsg):
    """This plugin provides debugging abilities for Supybot. It
    should not be loaded with a default installation."""
@@ -94,6 +100,7 @@ def eval(self, irc, msg, args, s):
        returns its value.  If an exception is raised, reports the
        exception (and logs the traceback to the bot's logfile).
        """
+        checkAllowShell(irc)
        try:
            self._evalEnv.update(locals())
            x = eval(s, self._evalEnv, self._evalEnv)
@@ -110,6 +117,7 @@ def _exec(self, irc, msg, args, s):

        Execs <code>.  Returns success if it didn't raise any exceptions.
        """
+        checkAllowShell(irc)
        exec(s)
        irc.replySuccess()
    _exec = wrap(_exec, ['text'])
@@ -119,6 +127,7 @@ def simpleeval(self, irc, msg, args, text):

        Evaluates the given expression.
        """
+        checkAllowShell(irc)
        try:
            irc.reply(repr(eval(text)))
        except Exception as e:
@@ -130,6 +139,7 @@ def exn(self, irc, msg, args, name):

        Raises the exception matching <exception name>.
        """
+        checkAllowShell(irc) # Just to be safe, but probably not needed.
        if isinstance(__builtins__, dict):
            exn = __builtins__[name]
        else:
@@ -152,6 +162,7 @@ def settrace(self, irc, msg, args, filename):
        Starts tracing function calls to <filename>.  If <filename> is not
        given, sys.stdout is used.  This causes much output.
        """
+        checkAllowShell(irc)
        if filename:
            fd = open(filename, 'a')
        else:
@@ -165,6 +176,7 @@ def unsettrace(self, irc, msg, args):

        Stops tracing function calls on stdout.
        """
+        checkAllowShell(irc)
        sys.settrace(None)
        irc.replySuccess()
    unsettrace = wrap(unsettrace)
@@ -195,6 +207,7 @@ def environ(self, irc, msg, args):

        Returns the environment of the supybot process.
        """
+        checkAllowShell(irc) # possibly some secret data in the env
        irc.reply(repr(os.environ))
    environ = wrap(environ)

diff --git a/plugins/Debug/test.py b/plugins/Debug/test.py
@@ -33,4 +33,21 @@ class DebugTestCase(PluginTestCase):
    plugins = ('Debug',)


+    def testShellForbidden(self):
+        self.assertResponse('debug eval 1+2', '3')
+        self.assertResponse('debug simpleeval 1+2', '3')
+        self.assertResponse('debug exec irc.reply(1+2)', '3')
+        while self.irc.takeMsg():
+            pass
+        self.assertNotError('debug environ')
+        with conf.supybot.commands.allowShell.context(False):
+            self.assertRegexp('debug eval 1+2',
+                    'Error:.*not available.*supybot.commands.allowShell')
+            self.assertRegexp('debug simpleeval 1+2',
+                    'Error:.*not available.*supybot.commands.allowShell')
+            self.assertRegexp('debug exec irc.reply(1+2)',
+                    'Error:.*not available.*supybot.commands.allowShell')
+            self.assertRegexp('debug environ',
+                    'Error:.*not available.*supybot.commands.allowShell')
+
 # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79:
diff --git a/plugins/Karma/plugin.py b/plugins/Karma/plugin.py
@@ -45,6 +45,11 @@

 import sqlite3

+def checkAllowShell(irc):
+    if not conf.supybot.commands.allowShell():
+        irc.error('This command is not available, because '
+            'supybot.commands.allowShell is False.', Raise=True)
+
 class SqliteKarmaDB(object):
    def __init__(self, filename):
        self.dbs = ircutils.IrcDict()
@@ -395,6 +400,7 @@ def dump(self, irc, msg, args, channel, filename):
        data directory.  <channel> is only necessary if the message isn't sent
        in the channel itself.
        """
+        checkAllowShell(irc)
        self.db.dump(channel, filename)
        irc.replySuccess()
    dump = wrap(dump, [('checkCapability', 'owner'), 'channeldb', 'filename'])
@@ -407,6 +413,7 @@ def load(self, irc, msg, args, channel, filename):
        data directory.  <channel> is only necessary if the message isn't sent
        in the channel itself.
        """
+        checkAllowShell(irc)
        self.db.load(channel, filename)
        irc.replySuccess()
    load = wrap(load, [('checkCapability', 'owner'), 'channeldb', 'filename'])
diff --git a/plugins/PluginDownloader/plugin.py b/plugins/PluginDownloader/plugin.py
@@ -359,6 +359,9 @@ def install(self, irc, msg, args, repository, plugin):
        """<repository> <plugin>

        Downloads and installs the <plugin> from the <repository>."""
+        if not conf.supybot.commands.allowShell():
+            irc.error(_('This command is not available, because '
+                'supybot.commands.allowShell is False.'), Raise=True)
        global repositories
        if repository not in repositories:
            irc.error(_(
diff --git a/plugins/PluginDownloader/test.py b/plugins/PluginDownloader/test.py
@@ -71,6 +71,11 @@ def testInstallProgVal(self):
        self.assertError('plugindownloader install ProgVal Darcs')
        self._testPluginInstalled('AttackProtector')

+    def testShellForbidden(self):
+        with conf.supybot.commands.allowShell.context(False):
+            self.assertRegexp('plugindownloader install ProgVal Darcs',
+                    'Error:.*not available.*supybot.commands.allowShell')
+
    def testInstallQuantumlemur(self):
        self.assertError('plugindownloader install quantumlemur AttackProtector')
        self.assertNotError('plugindownloader install quantumlemur Listener')
diff --git a/plugins/Unix/plugin.py b/plugins/Unix/plugin.py
@@ -40,6 +40,7 @@
 import subprocess
 import shlex

+import supybot.conf as conf
 import supybot.utils as utils
 from supybot.commands import *
 import supybot.utils.minisix as minisix
@@ -50,6 +51,11 @@
 from supybot.i18n import PluginInternationalization, internationalizeDocstring
 _ = PluginInternationalization('Unix')

+def checkAllowShell(irc):
+    if not conf.supybot.commands.allowShell():
+        irc.error(_('This command is not available, because '
+            'supybot.commands.allowShell is False.'), Raise=True)
+
 _progstats_endline_remover = utils.str.MultipleRemover('\r\n')
 def progstats():
    pw = pwd.getpwuid(os.getuid())
@@ -401,6 +407,7 @@ def call(self, irc, msg, args, text):
        you don't run anything that will spamify your channel or that
        will bring your machine to its knees.
        """
+        checkAllowShell(irc)
        self.log.info('Unix: running command "%s" for %s/%s', text, msg.nick,
                      irc.network)
        args = shlex.split(text)
@@ -433,6 +440,7 @@ def shell(self, irc, msg, args, text):
        you don't run anything that will spamify your channel or that
        will bring your machine to its knees.
        """
+        checkAllowShell(irc)
        self.log.info('Unix: running command "%s" for %s/%s', text, msg.nick,
                      irc.network)
        try:
diff --git a/plugins/Unix/test.py b/plugins/Unix/test.py
@@ -172,6 +172,12 @@ def testCall(self):
            self.assertRegexp('unix call /bin/ls /', 'boot, .*dev, ')
            self.assertError('unix call /usr/bin/nosuchcommandaoeuaoeu')

+        def testShellForbidden(self):
+            self.assertNotError('unix call /bin/ls /')
+            with conf.supybot.commands.allowShell.context(False):
+                self.assertRegexp('unix call /bin/ls /',
+                        'Error:.*not available.*supybot.commands.allowShell')
+
        def testUptime(self):
            self.assertNotError('unix sysuptime')

diff --git a/src/conf.py b/src/conf.py
@@ -748,6 +748,16 @@ class ValidBrackets(registry.OnlySomeStrings):
        know what you're doing, then also know that this set is
        case-sensitive.""")))

+# For this config variable to make sense, it must no be writable via IRC.
+# Make sure it is always blacklisted from the Config plugin.
+registerGlobalValue(supybot.commands, 'allowShell',
+    registry.Boolean(True, _("""Allows this bot's owner user to use commands
+    that grants them shell access. This config variable exists in case you want
+    to prevent MITM from the IRC network itself (vulnerable IRCd or IRCops)
+    from gaining shell access to the bot's server by impersonating the owner.
+    Setting this to False also disables plugins and commands that can be
+    used to indirectly gain shell access.""")))
+
 # supybot.commands.disabled moved to callbacks for canonicalName.

 ###