rule parsing: fix infinite loop on missing ;

If the last rule option was missing a ";" the parser would
enter an infinite loop. Instead error out with an exception
that can be logged.

Test case added.

From an reported on the idstools rule parser.

diff --git a/suricata/update/rule.py b/suricata/update/rule.py
index d9be2b38c3f483ab4311c8921b133b1c7748992d..3487443092f6755e5760ba834407c5b787c92e25 100644 (file)
--- a/suricata/update/rule.py
+++ b/suricata/update/rule.py
@@ -70,6 +70,11 @@ decoder_rule_pattern = re.compile(
     r")"
     % "|".join(actions))
 
+class NoEndOfOptionError(Exception):
+    """Exception raised when the end of option terminator (semicolon) is
+    missing."""
+    pass
+
 class Rule(dict):
     """Class representing a rule.
 
@@ -233,6 +238,8 @@ def parse(buf, group=None):
         if not options:
             break
         index = find_opt_end(options)
+        if index < 0:
+            raise NoEndOfOptionError("no end of option")
         option = options[:index].strip()
         options = options[index + 1:].strip()
 
@@ -291,13 +298,13 @@ def parse_fileobj(fileobj, group=None):
         if line.rstrip().endswith("\\"):
             buf = "%s%s " % (buf, line.rstrip()[0:-1])
             continue
+        buf = buf + line
         try:
-            rule = parse(buf + line, group)
+            rule = parse(buf, group)
             if rule:
                 rules.append(rule)
-        except:
-            logger.error("failed to parse rule: %s" % (buf))
-            raise
+        except Exception as err:
+            logger.error("Failed to parse rule: %s: %s", buf.rstrip(), err)
         buf = ""
     return rules
 

diff --git a/tests/test_rule.py b/tests/test_rule.py
index c13c1848a667bc78a1445dec74e7bd002181dc56..5f8af046bc37b9282cdd057af9982707a6642f43 100644 (file)
--- a/tests/test_rule.py
+++ b/tests/test_rule.py
@@ -185,3 +185,11 @@ alert dnp3 any any -> any any (msg:"SURICATA DNP3 Request flood detected"; \
         self.assertIsNotNone(rule)
         self.assertTrue("former_category TROJAN" in rule.metadata)
         self.assertTrue("updated_at 2017_08_08" in rule.metadata)
+
+    def test_parse_option_missing_end(self):
+        """Test parsing a rule where the last option is missing a
+        semicolon. This was responsible for an infinite loop. """
+        rule_buf = u"""alert icmp any any -> $HOME_NET any (msg:"ICMP test detected"; gid:0; sid:10000001; rev:1; classtype: icmp-event; metadata:policy balanced-ips drop, policy connectivity-ips drop, policy security-ips drop)"""
+        self.assertRaises(
+            suricata.update.rule.NoEndOfOptionError,
+            suricata.update.rule.parse, rule_buf)