From: Jason Ish <ish@unx.ca>
Date: Mon, 20 Nov 2017 21:15:54 +0000 (-0600)
Subject: rule parsing: fix infinite loop on missing ;
X-Git-Tag: 1.0.0a1~40
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6f4cd19c5e27105143558d00e8950f09d12e421e;p=thirdparty%2Fsuricata-update.git

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 d9be2b3..3487443 100644
--- 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 c13c184..5f8af04 100644
--- 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)