Merge pull request #904 from greenbone/update_grammar

Change: Extend Grammar plugin regex. Add relevant test cases.

Author: cfi-gb

tests/plugins/test_grammar.py

@@ -328,6 +328,58 @@ def test_grammar10(self):
             results[0].message,
         )
 
+    def test_grammar11(self):
+        nasl_file = Path(__file__).parent / "test.nasl"
+        content = (
+            '  script_tag(name:"cvss_base", value:"4.0");\n'
+            '  script_tag(name:"impact", value:"Inadequate checks in '
+            "com_contact could allowed mail submission\n"
+            '  script_tag(name:"solution_type", value:"VendorFix");\n'
+        )
+
+        fake_context = self.create_file_plugin_context(
+            nasl_file=nasl_file, file_content=content
+        )
+        plugin = CheckGrammar(fake_context)
+
+        results = list(plugin.run())
+
+        self.assertEqual(len(results), 1)
+        self.assertIsInstance(results[0], LinterError)
+        self.assertEqual(
+            "VT/Include has the following grammar problem:\n"
+            "- Hit: could allowed\n"
+            '- Full line: script_tag(name:"impact", value:"Inadequate checks '
+            "in com_contact could allowed mail submission",
+            results[0].message,
+        )
+
+    def test_grammar12(self):
+        nasl_file = Path(__file__).parent / "test.nasl"
+        content = (
+            '  script_tag(name:"cvss_base", value:"4.0");\n'
+            '  script_tag(name:"impact", value:"This allow an attacker to gain '
+            "administrative access to the\n"
+            '  script_tag(name:"solution_type", value:"VendorFix");\n'
+        )
+
+        fake_context = self.create_file_plugin_context(
+            nasl_file=nasl_file, file_content=content
+        )
+        plugin = CheckGrammar(fake_context)
+
+        results = list(plugin.run())
+
+        self.assertEqual(len(results), 1)
+        self.assertIsInstance(results[0], LinterError)
+        self.assertEqual(
+            "VT/Include has the following grammar problem:\n"
+            "- Hit: This allow\n"
+            '- Full line: script_tag(name:"impact", value:"This allow an '
+            "attacker to gain administrative access to the",
+            results[0].message,
+        )
+
     def test_grammar_fp(self):
         nasl_file = Path(__file__).parent / "test.nasl"
         content = (
@@ -383,3 +435,21 @@ def test_grammar_fp2(self):
         results = list(plugin.run())
 
         self.assertEqual(len(results), 0)
+
+    def test_grammar_fp3(self):
+        nasl_file = Path(__file__).parent / "test.nasl"
+        content = (
+            '  script_tag(name:"cvss_base", value:"4.0");\n'
+            '  script_tag(name:"insight", value:"*snip* connection string to '
+            'provide\nproperties that are not on this allow list.");\n'
+            '  script_tag(name:"solution_type", value:"VendorFix");\n'
+            '  script_tag(name:"solution", value:"meh");\n'
+        )
+        fake_context = self.create_file_plugin_context(
+            nasl_file=nasl_file, file_content=content
+        )
+        plugin = CheckGrammar(fake_context)
+
+        results = list(plugin.run())
+
+        self.assertEqual(len(results), 0)

troubadix/plugins/grammar.py

@@ -43,8 +43,13 @@
     # From several Ubuntu LSCs like e.g.:
     # 2021/ubuntu/gb_ubuntu_USN_4711_1.nasl
     TextCheck("An attacker with access to at least one LUN in a multiple"),
-    # nb: The regex to catch "this files" might catch this wrongly...
-    PatternCheck(r"th(is|ese)\s+filesystem", re.IGNORECASE),
+    # nb:
+    # - The regex to catch e.g. "this files" or "This allow an attacker" might
+    #   catch this wrongly...
+    # - Cases like "this filesystem" vs. "these filesystems" are also handled /
+    #   excluded here
+    PatternCheck(r'this\s+(filesystem|allow\s+list)[\s.",]+', re.IGNORECASE),
+    PatternCheck(r'these\s+(filesystem|allow\s+list)s[\s.",]+', re.IGNORECASE),
     # Like seen in e.g. 2008/freebsd/freebsd_mod_php4-twig.nasl
     PatternCheck(r'(\s+|")[Aa]\s+multiple\s+of'),
     # WITH can be used like e.g. the following which is valid:
@@ -183,6 +188,12 @@ def get_grammer_pattern() -> re.Pattern:
         # Successful exploitation may allows an attacker to run arbitrary
         # An error in INSTALL_JAR procedure might allows remote authenticated
         r"(could|may|will|might|should|can)\s+allows\s+|"
+        # e.g.:
+        # - Inadequate checks in com_contact could allowed mail submission
+        r"(could|may|will|might|should|can)\s+allowed\s+|"
+        # e.g.:
+        # This allow an attacker to gain administrative access to the
+        r"This\s+allow\s+|"
         # nb: Next few could happen when copy'n'paste some text parts around
         # like e.g.:
         # is prone to a to a remote denial-of-service vulnerability