github
/
fail2ban
mirror of https://github.com/fail2ban/fail2ban
1
0
Fork 0
Code Issues Releases Wiki Activity

[interim commit] try to fix possible escape vulnerability in actions

pull/1726/head
sebres 2017-03-15 16:35:40 +01:00
parent 93ec9e01d4
commit e5c9f9ec1c
5 changed files with 119 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
fail2ban/server/action.py
View File

@ -453,7 +453,7 @@ class CommandAction(ActionBase):
return value
@classmethod
def replaceTag(cls, query, aInfo, conditional='', cache=None, substRec=True):
def replaceTag(cls, query, aInfo, conditional='', cache=None):
"""Replaces tags in `query` with property values.
Parameters
@ -481,9 +481,8 @@ class CommandAction(ActionBase):
# **Important**: don't replace if calling map - contains dynamic values only,
# no recursive tags, otherwise may be vulnerable on foreign user-input:
noRecRepl = isinstance(aInfo, CallingMap)
if noRecRepl:
subInfo = aInfo
else:
subInfo = aInfo
if not noRecRepl:
# substitute tags recursive (and cache if possible),
# first try get cached tags dictionary:
subInfo = csubkey = None
@ -534,13 +533,50 @@ class CommandAction(ActionBase):
"unexpected too long replacement interpolation, "
"possible self referencing definitions in query: %s" % (query,))
# cache if possible:
if cache is not None:
cache[ckey] = value
#
return value
@classmethod
def replaceDynamicTags(cls, realCmd, aInfo):
"""Replaces dynamical tags in `query` with property values.
**Important**
-------------
Because this tags are dynamic resp. foreign (user) input:
- values should be escaped
- no recursive substitution (no interpolation for <a<b>>)
- don't use cache
Parameters
----------
query : str
String with tags.
aInfo : dict
Tags(keys) and associated values for substitution in query.
Returns
-------
str
shell script as string or array with tags replaced (direct or as variables).
"""
realCmd = cls.replaceTag(realCmd, aInfo, conditional=False)
# Replace ticket options (filter capture groups) non-recursive:
if '<' in realCmd:
tickData = aInfo.get("F-*")
if not tickData: tickData = {}
def substTag(m):
tn = mapTag2Opt(m.groups()[0])
try:
return str(tickData[tn])
except KeyError:
return ""
realCmd = FCUSTAG_CRE.sub(substTag, realCmd)
return realCmd
def _processCmd(self, cmd, aInfo=None, conditional=''):
"""Executes a command with preliminary checks and substitutions.
@ -605,21 +641,9 @@ class CommandAction(ActionBase):
realCmd = self.replaceTag(cmd, self._properties,
conditional=conditional, cache=self.__substCache)
# Replace dynamical tags (don't use cache here)
# Replace dynamical tags, important - don't cache, no recursion and auto-escape here
if aInfo is not None:
realCmd = self.replaceTag(realCmd, aInfo, conditional=conditional)
# Replace ticket options (filter capture groups) non-recursive:
if '<' in realCmd:
tickData = aInfo.get("F-*")
if not tickData: tickData = {}
def substTag(m):
tn = mapTag2Opt(m.groups()[0])
try:
return str(tickData[tn])
except KeyError:
return ""
realCmd = FCUSTAG_CRE.sub(substTag, realCmd)
realCmd = self.replaceDynamicTags(realCmd, aInfo)
else:
realCmd = cmd
@ -653,8 +677,5 @@ class CommandAction(ActionBase):
logSys.debug("Nothing to do")
return True
_cmd_lock.acquire()
try:
with _cmd_lock:
return Utils.executeCmd(realCmd, timeout, shell=True, output=False, **kwargs)
finally:
_cmd_lock.release()

1
fail2ban/server/actions.py
View File

@ -290,6 +290,7 @@ class Actions(JailThread, Mapping):
AI_DICT = {
"ip": lambda self: self.__ticket.getIP(),
"family": lambda self: self['ip'].familyStr,
"ip-rev": lambda self: self['ip'].getPTR(''),
"ip-host": lambda self: self['ip'].getHost(),
"fid": lambda self: self.__ticket.getID(),

5
fail2ban/server/ipdns.py
View File

@ -261,6 +261,11 @@ class IPAddr(object):
def family(self):
return self._family
FAM2STR = {socket.AF_INET: 'inet4', socket.AF_INET6: 'inet6'}
@property
def familyStr(self):
return IPAddr.FAM2STR.get(self._family)
@property
def plen(self):
return self._plen

64
fail2ban/server/utils.py
View File

@ -28,7 +28,7 @@ import signal
import subprocess
import sys
import time
from ..helpers import getLogger, uni_decode
from ..helpers import getLogger, _merge_dicts, uni_decode
if sys.version_info >= (3, 3):
import importlib.machinery
@ -116,7 +116,22 @@ class Utils():
return flags
@staticmethod
def executeCmd(realCmd, timeout=60, shell=True, output=False, tout_kill_tree=True, success_codes=(0,)):
def buildShellCmd(realCmd, varsDict):
# build map as array of vars and command line array:
varsStat = ""
if not isinstance(realCmd, list):
realCmd = [realCmd]
i = len(realCmd)-1
for k, v in varsDict.iteritems():
varsStat += "%s=$%s " % (k, i)
realCmd.append(v)
i += 1
realCmd[0] = varsStat + "\n" + realCmd[0]
return realCmd
@staticmethod
def executeCmd(realCmd, timeout=60, shell=True, output=False, tout_kill_tree=True,
success_codes=(0,), varsDict=None):
"""Executes a command.
Parameters
@ -131,6 +146,8 @@ class Utils():
output : bool
If output is True, the function returns tuple (success, stdoutdata, stderrdata, returncode).
If False, just indication of success is returned
varsDict: dict
variables supplied to the command (or to the shell script)
Returns
-------
@ -146,10 +163,18 @@ class Utils():
"""
stdout = stderr = None
retcode = None
popen = None
popen = env = None
if varsDict:
if shell:
# build map as array of vars and command line array:
realCmd = Utils.buildShellCmd(realCmd, varsDict)
else: # pragma: no cover - currently unused
env = _merge_dicts(os.environ, varsDict)
realCmdId = id(realCmd)
outCmd = lambda level: logSys.log(level, "%x -- exec: %s", realCmdId, realCmd)
try:
popen = subprocess.Popen(
realCmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=shell,
realCmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=shell, env=env,
preexec_fn=os.setsid # so that killpg does not kill our process
)
# wait with timeout for process has terminated:
@ -158,13 +183,15 @@ class Utils():
def _popen_wait_end():
retcode = popen.poll()
return (True, retcode) if retcode is not None else None
retcode = Utils.wait_for(_popen_wait_end, timeout, Utils.DEFAULT_SHORT_INTERVAL)
# popen.poll is fast operation so we can put down the sleep interval:
retcode = Utils.wait_for(_popen_wait_end, timeout, Utils.DEFAULT_SHORT_INTERVAL / 100)
if retcode:
retcode = retcode[1]
# if timeout:
if retcode is None:
logSys.error("%s -- timed out after %s seconds." %
(realCmd, timeout))
if outCmd: outCmd(logging.ERROR); outCmd = None
logSys.error("%x -- timed out after %s seconds." %
(realCmdId, timeout))
pgid = os.getpgid(popen.pid)
# if not tree - first try to terminate and then kill, otherwise - kill (-9) only:
os.killpg(pgid, signal.SIGTERM) # Terminate the process
@ -185,48 +212,49 @@ class Utils():
return False if not output else (False, stdout, stderr, retcode)
std_level = logging.DEBUG if retcode in success_codes else logging.ERROR
if std_level > logSys.getEffectiveLevel():
if outCmd: outCmd(std_level-1); outCmd = None
# if we need output (to return or to log it):
if output or std_level >= logSys.getEffectiveLevel():
# if was timeouted (killed/terminated) - to prevent waiting, set std handles to non-blocking mode.
if popen.stdout:
try:
if retcode is None or retcode < 0:
Utils.setFBlockMode(popen.stdout, False)
stdout = popen.stdout.read()
except IOError as e:
except IOError as e: # pragma: no cover
logSys.error(" ... -- failed to read stdout %s", e)
if stdout is not None and stdout != '' and std_level >= logSys.getEffectiveLevel():
logSys.log(std_level, "%s -- stdout:", realCmd)
for l in stdout.splitlines():
logSys.log(std_level, " -- stdout: %r", uni_decode(l))
logSys.log(std_level, "%x -- stdout: %r", realCmdId, uni_decode(l))
popen.stdout.close()
if popen.stderr:
try:
if retcode is None or retcode < 0:
Utils.setFBlockMode(popen.stderr, False)
stderr = popen.stderr.read()
except IOError as e:
except IOError as e: # pragma: no cover
logSys.error(" ... -- failed to read stderr %s", e)
if stderr is not None and stderr != '' and std_level >= logSys.getEffectiveLevel():
logSys.log(std_level, "%s -- stderr:", realCmd)
for l in stderr.splitlines():
logSys.log(std_level, " -- stderr: %r", uni_decode(l))
logSys.log(std_level, "%x -- stderr: %r", realCmdId, uni_decode(l))
popen.stderr.close()
success = False
if retcode in success_codes:
logSys.debug("%-.40s -- returned successfully %i", realCmd, retcode)
logSys.debug("%x -- returned successfully %i", realCmdId, retcode)
success = True
elif retcode is None:
logSys.error("%-.40s -- unable to kill PID %i", realCmd, popen.pid)
logSys.error("%x -- unable to kill PID %i", realCmdId, popen.pid)
elif retcode < 0 or retcode > 128:
# dash would return negative while bash 128 + n
sigcode = -retcode if retcode < 0 else retcode - 128
logSys.error("%-.40s -- killed with %s (return code: %s)",
realCmd, signame.get(sigcode, "signal %i" % sigcode), retcode)
logSys.error("%x -- killed with %s (return code: %s)",
realCmdId, signame.get(sigcode, "signal %i" % sigcode), retcode)
else:
msg = _RETCODE_HINTS.get(retcode, None)
logSys.error("%-.40s -- returned %i", realCmd, retcode)
logSys.error("%x -- returned %i", realCmdId, retcode)
if msg:
logSys.info("HINT on %i: %s", retcode, msg % locals())
if output:

28
fail2ban/tests/actiontestcase.py
View File

@ -389,6 +389,23 @@ class CommandActionTest(LogCaptureTestCase):
self.assertLogged('Nothing to do')
self.pruneLog()
def testExecuteWithVars(self):
self.assertTrue(self.__action.executeCmd(
r'''printf %b "foreign input:\n'''
r''' -- $f2bV_A --\n'''
r''' -- $f2bV_B --\n'''
r''' -- $(echo $f2bV_C) --''' # echo just replaces \n to test it as single line
r'''"''',
varsDict={
'f2bV_A': 'I\'m a hacker; && $(echo $f2bV_B)',
'f2bV_B': 'I"m very bad hacker',
'f2bV_C': '`Very | very\n$(bad & worst hacker)`'
}))
self.assertLogged(r"""foreign input:""",
' -- I\'m a hacker; && $(echo $f2bV_B) --',
' -- I"m very bad hacker --',
' -- `Very | very $(bad & worst hacker)` --', all=True)
def testExecuteIncorrectCmd(self):
CommandAction.executeCmd('/bin/ls >/dev/null\nbogusXXX now 2>/dev/null')
self.assertLogged('HINT on 127: "Command not found"')
@ -400,8 +417,9 @@ class CommandActionTest(LogCaptureTestCase):
self.assertFalse(CommandAction.executeCmd('sleep 30', timeout=timeout))
# give a test still 1 second, because system could be too busy
self.assertTrue(time.time() >= stime + timeout and time.time() <= stime + timeout + 1)
self.assertLogged('sleep 30 -- timed out after')
self.assertLogged('sleep 30 -- killed with SIGTERM')
self.assertLogged('sleep 30', ' -- timed out after', all=True)
self.assertLogged(' -- killed with SIGTERM',
' -- killed with SIGKILL')
def testExecuteTimeoutWithNastyChildren(self):
# temporary file for a nasty kid shell script
@ -457,9 +475,9 @@ class CommandActionTest(LogCaptureTestCase):
# Verify that the process itself got killed
self.assertTrue(Utils.wait_for(lambda: not pid_exists(cpid), 3))
self.assertLogged('my pid ', 'Resource temporarily unavailable')
self.assertLogged('timed out')
self.assertLogged('killed with SIGTERM',
'killed with SIGKILL')
self.assertLogged(' -- timed out')
self.assertLogged(' -- killed with SIGTERM',
' -- killed with SIGKILL')
os.unlink(tmpFilename)
os.unlink(tmpFilename + '.pid')