Permalink
Browse files

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

  • Loading branch information...
1 parent 82cdf17 commit 4f6a5e7db9ee586120fda8138b5e3210ad54a011 @ProgVal committed Sep 24, 2017
View
@@ -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'])
View
@@ -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:
View
@@ -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)
View
@@ -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:
View
@@ -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'])
@@ -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(_(
@@ -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')
View
@@ -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:
View
@@ -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')
View
@@ -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.
###

0 comments on commit 4f6a5e7

Please sign in to comment.