From 93f776d2eec1ae962642372e8f522dafc7eebb7d Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 25 May 2016 21:33:26 +0200 Subject: [PATCH] amend for gh-1419: tags substitution bug - wrong recognition of cyclic recursion, new test cases covered this --- fail2ban/server/action.py | 28 ++++++++------- fail2ban/tests/actiontestcase.py | 62 +++++++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/fail2ban/server/action.py b/fail2ban/server/action.py index 51c5e077..f579c31e 100644 --- a/fail2ban/server/action.py +++ b/fail2ban/server/action.py @@ -46,9 +46,13 @@ _cmd_lock = threading.Lock() # Todo: make it configurable resp. automatically set, ex.: `[ -f /proc/net/if_inet6 ] && echo 'yes' || echo 'no'`: allowed_ipv6 = True +# max tag replacement count: +MAX_TAG_REPLACE_COUNT = 10 + # compiled RE for tag name (replacement name) TAG_CRE = re.compile(r'<([^ <>]+)>') + class CallingMap(MutableMapping): """A Mapping type which returns the result of callable values. @@ -356,32 +360,28 @@ class CommandAction(ActionBase): tags = inptags.copy() t = TAG_CRE # repeat substitution while embedded-recursive (repFlag is True) + done = cls._escapedTags.copy() while True: repFlag = False # substitute each value: for tag in tags.iterkeys(): - if tag in cls._escapedTags: - # Escaped so won't match - continue + # ignore escaped or already done: + if tag in done: continue value = str(tags[tag]) # search and replace all tags within value, that can be interpolated using other tags: m = t.search(value) - done = {} - last_found = tag + refCounts = {} #logSys.log(5, 'TAG: %s, value: %s' % (tag, value)) while m: found_tag = m.group(1) #logSys.log(5, 'found: %s' % found_tag) - curdone = done.get(last_found) - if curdone is None: - done[last_found] = curdone = [] - if found_tag == tag or found_tag in curdone: + if found_tag == tag or refCounts.get(found_tag, 1) > MAX_TAG_REPLACE_COUNT: # recursive definitions are bad #logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) ) raise ValueError( "properties contain self referencing definitions " "and cannot be resolved, fail tag: %s, found: %s in %s, value: %s" % - (tag, found_tag, curdone, value)) + (tag, found_tag, refCounts, value)) repl = None if found_tag not in cls._escapedTags: repl = tags.get(found_tag + '?' + conditional) @@ -395,8 +395,9 @@ class CommandAction(ActionBase): continue value = value.replace('<%s>' % found_tag, repl) #logSys.log(5, 'value now: %s' % value) - curdone.append(found_tag) - last_found = found_tag + # increment reference count: + refCounts[found_tag] = refCounts.get(found_tag, 0) + 1 + # the next match for replace: m = t.search(value, m.start()) #logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value)) # was substituted? @@ -405,6 +406,9 @@ class CommandAction(ActionBase): if t.search(value): repFlag = True tags[tag] = value + # no more sub tags (and no possible composite), add this tag to done set (just to be faster): + if '<' not in value: done.add(tag) + # stop interpolation, if no replacements anymore: if not repFlag: break return tags diff --git a/fail2ban/tests/actiontestcase.py b/fail2ban/tests/actiontestcase.py index ac59a400..457d81bd 100644 --- a/fail2ban/tests/actiontestcase.py +++ b/fail2ban/tests/actiontestcase.py @@ -66,18 +66,62 @@ class CommandActionTest(LogCaptureTestCase): lambda: CommandAction.substituteRecursiveTags({'A': 'to= fromip=', 'C': '', 'B': '', 'D': ''})) self.assertRaises(ValueError, lambda: CommandAction.substituteRecursiveTags({'failregex': 'to= fromip=', 'sweet': '', 'honeypot': '', 'ignoreregex': ''})) - # No-recursion, just multiple replacement of tag , should be successful - if OrderedDict: # we need here an ordered, because the sequence of iteration is very important for this test - self.assertEqual(CommandAction.substituteRecursiveTags( - OrderedDict((('X', 'x=x'), ('T', '1'), ('Z', ' '), ('Y', 'y=y'))) + # We need here an ordered, because the sequence of iteration is very important for this test + if OrderedDict: + # No cyclic recursion, just multiple replacement of tag , should be successful: + self.assertEqual(CommandAction.substituteRecursiveTags( OrderedDict( + (('X', 'x=x'), ('T', '1'), ('Z', ' '), ('Y', 'y=y'))) ), {'X': 'x=x1', 'T': '1', 'Y': 'y=y1', 'Z': 'x=x1 1 y=y1'} ) - # No-recursion, just multiple replacement of tag , should be successful - if OrderedDict: # we need here an ordered, because the sequence of iteration is very important for this test - self.assertEqual(CommandAction.substituteRecursiveTags( - OrderedDict((('X', 'x=x'), ('T', '1'), ('Z', ' '), ('Y', 'y=y'))) - ), {'X': 'x=x1', 'T': '1', 'Y': 'y=y1', 'Z': 'x=x1 1 y=y1'} + # No cyclic recursion, just multiple replacement of tag in composite tags, should be successful: + self.assertEqual(CommandAction.substituteRecursiveTags( OrderedDict( + (('X', 'x=x <> <>'), ('R1', 'Z'), ('R2', 'Y'), ('T', '1'), ('Z', ' '), ('Y', 'y=y'))) + ), {'X': 'x=x1 1 y=y1 1 y=y1 y=y1', 'R1': 'Z', 'R2': 'Y', 'T': '1', 'Z': '1 y=y1', 'Y': 'y=y1'} ) + # No cyclic recursion, just multiple replacement of same tags, should be successful: + self.assertEqual(CommandAction.substituteRecursiveTags( OrderedDict(( + ('actionstart', 'ipset create hash:ip timeout family \n -I '), + ('ipmset', 'f2b-'), + ('name', 'any'), + ('bantime', '600'), + ('ipsetfamily', 'inet'), + ('iptables', 'iptables '), + ('lockingopt', '-w'), + ('chain', 'INPUT'), + ('actiontype', ''), + ('multiport', '-p -m multiport --dports -m set --match-set src -j '), + ('protocol', 'tcp'), + ('port', 'ssh'), + ('blocktype', 'REJECT',), + )) + ), OrderedDict(( + ('actionstart', 'ipset create f2b-any hash:ip timeout 600 family inet\niptables -w -I INPUT -p tcp -m multiport --dports ssh -m set --match-set f2b-any src -j REJECT'), + ('ipmset', 'f2b-any'), + ('name', 'any'), + ('bantime', '600'), + ('ipsetfamily', 'inet'), + ('iptables', 'iptables -w'), + ('lockingopt', '-w'), + ('chain', 'INPUT'), + ('actiontype', '-p tcp -m multiport --dports ssh -m set --match-set f2b-any src -j REJECT'), + ('multiport', '-p tcp -m multiport --dports ssh -m set --match-set f2b-any src -j REJECT'), + ('protocol', 'tcp'), + ('port', 'ssh'), + ('blocktype', 'REJECT') + )) + ) + # Cyclic recursion by composite tag creation, tags "create" another tag, that closes cycle: + self.assertRaises(ValueError, lambda: CommandAction.substituteRecursiveTags( OrderedDict(( + ('A', '<>'), + ('B', 'D'), ('C', 'E'), + ('DE', 'cycle '), + )) )) + self.assertRaises(ValueError, lambda: CommandAction.substituteRecursiveTags( OrderedDict(( + ('DE', 'cycle '), + ('A', '<>'), + ('B', 'D'), ('C', 'E'), + )) )) + # missing tags are ok self.assertEqual(CommandAction.substituteRecursiveTags({'A': ''}), {'A': ''}) self.assertEqual(CommandAction.substituteRecursiveTags({'A': ' ','X':'fun'}), {'A': ' fun', 'X':'fun'})