Fix TypeError : Failure with missing rule params

Bug #2867 : Failure with missing rule params
If sometimes a rule is not parsed correctly and sid or gid are of
NoneType the program shows a TypeError. While parsing the rule, the sid
can be checked to make sure that sid is not of type None, and if it is,
raise a BadSidError and log it, the rule is not added to the list of
final rules.

diff --git a/suricata/update/rule.py b/suricata/update/rule.py
index 781d8d8d5125f3683f48a9853b6d255bb5582853..7f6bab5552afd6065cbc6f1b1d00a35753623050 100644 (file)
--- a/suricata/update/rule.py
+++ b/suricata/update/rule.py
@@ -161,6 +161,9 @@ def find_opt_end(options):
         else:
             return offset + i
 
+class BadSidError(Exception):
+    """Raises exception when sid is of type null"""
+
 def parse(buf, group=None):
     """ Parse a single rule for a string buffer.
 
@@ -286,6 +289,9 @@ def parse(buf, group=None):
     if rule["msg"] is None:
         rule["msg"] = ""
 
+    if not rule["sid"]:
+        raise BadSidError("Sid cannot be of type null")
+
     rule["raw"] = m.group("raw").strip()
 
     return rule

diff --git a/tests/test_rule.py b/tests/test_rule.py
index d94043e4d063b11a707e661ece96100a07ab272b..ef7ee6cd69dbe72e408c535c347053fd075dda8b 100644 (file)
--- a/tests/test_rule.py
+++ b/tests/test_rule.py
@@ -46,34 +46,35 @@ class RuleTestCase(unittest.TestCase):
         self.assertEqual(rule.classtype, "trojan-activity")
 
     def test_disable_rule(self):
-        rule_buf = """# alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)"""
+        rule_buf = """# alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)"""
         rule = suricata.update.rule.parse(rule_buf)
         self.assertFalse(rule.enabled)
-        self.assertEqual(rule.raw, """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)""")
+        self.assertEqual(rule.raw, """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)""")
         self.assertEqual(str(rule), rule_buf)
 
     def test_parse_rule_double_commented(self):
-        rule_buf = """## alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)"""
+        rule_buf = """## alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)"""
         rule = suricata.update.rule.parse(rule_buf)
         self.assertFalse(rule.enabled)
-        self.assertEqual(rule.raw, """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)""")
+        self.assertEqual(rule.raw, """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)""")
 
     def test_parse_rule_comments_and_spaces(self):
-        rule_buf = """## #alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)"""
+        rule_buf = """## #alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)"""
         rule = suricata.update.rule.parse(rule_buf)
         self.assertFalse(rule.enabled)
-        self.assertEqual(rule.raw, """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)""")
+        self.assertEqual(rule.raw, """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)""")
 
     def test_toggle_rule(self):
-        rule_buf = """# alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)"""
+        rule_buf = """# alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)"""
         rule = suricata.update.rule.parse(rule_buf)
         self.assertFalse(rule.enabled)
         rule.enabled = True
-        self.assertEqual(str(rule), """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";)""")
+        self.assertEqual(str(rule), """alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message"; sid:1;)""")
 
     def test_parse_fileobj(self):
-        rule_buf = ("""# alert tcp $HOME_NET any -> $EXTERNAL_NET any """
-                    """(msg:"some message";)""")
+        rule_buf = ("""alert ( msg:"DECODE_NOT_IPV4_DGRAM" sid:3; gid:116; rev:1; metadata:rule-type decode; classtype:protocol-command-decode;) \n"""
+                    """# alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";) \n"""
+                    """alert ( msg:"DECODE_NOT_IPV4_DGRAM"; sid:2; gid:116; rev:1; metadata:rule-type decode; classtype:protocol-command-decode;)""")
         fileobj = io.StringIO()
         for i in range(2):
             fileobj.write(u"%s\n" % rule_buf)
@@ -82,8 +83,9 @@ class RuleTestCase(unittest.TestCase):
         self.assertEqual(2, len(rules))
 
     def test_parse_file(self):
-        rule_buf = ("""# alert tcp $HOME_NET any -> $EXTERNAL_NET any """
-                    """(msg:"some message";)""")
+        rule_buf = ("""# alert tcp $HOME_NET any -> $EXTERNAL_NET any (msg:"some message";) \n"""
+                    """alert ( msg:"DECODE_NOT_IPV4_DGRAM"; sid:1; gid:116; rev:1; metadata:rule-type decode; classtype:protocol-command-decode;) \n"""
+                    """alert ( msg:"DECODE_NOT_IPV4_DGRAM" sid:1; gid:116; rev:1; metadata:rule-type decode; classtype:protocol-command-decode;) \n""")
         tmp = tempfile.NamedTemporaryFile()
         for i in range(2):
             tmp.write(("%s\n" % rule_buf).encode())
@@ -233,3 +235,10 @@ alert dnp3 any any -> any any (msg:"SURICATA DNP3 Request flood detected"; \
         rule_string = u"""alert ip any any -> any any (content:"uid=0|28|root|29|"; classtype:bad-unknown; sid:10000000;)"""
         rule = suricata.update.rule.parse(rule_string)
         self.assertEqual(0, rule["rev"])
+
+    def test_parse_no_sid(self):
+        """Test parsing a rule where the sid is not parsed correctly. """
+        rule_buf = u"""alert icmp any any -> $HOME_NET any (msg:"ICMP test detected"; gid:0; rev:1; classtype: icmp-event;)"""
+        self.assertRaises(
+            suricata.update.rule.BadSidError,
+            suricata.update.rule.parse, rule_buf)