From 46e2f069c235309a1283e010a255c98575205b87 Mon Sep 17 00:00:00 2001 From: Christian Fischer Date: Thu, 12 Oct 2023 15:33:23 +0200 Subject: [PATCH 1/6] Change: Extend script_tag_whitespaces to also check script_xref() tags. --- troubadix/plugins/script_tag_whitespaces.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/troubadix/plugins/script_tag_whitespaces.py b/troubadix/plugins/script_tag_whitespaces.py index fc80e75c..8447f4f4 100644 --- a/troubadix/plugins/script_tag_whitespaces.py +++ b/troubadix/plugins/script_tag_whitespaces.py @@ -24,6 +24,7 @@ from troubadix.helper.patterns import ( _get_special_script_tag_pattern, _get_tag_pattern, + get_xref_pattern, ) from troubadix.plugin import FileContentPlugin, LinterError, LinterResult @@ -51,7 +52,11 @@ def check_content( name=SpecialScriptTag.NAME.value ).finditer(file_content) - matches = chain(tag_matches, name_matches) + xref_matches = get_xref_pattern(name=r".+?", flags=re.S).finditer( + file_content + ) + + matches = chain(tag_matches, name_matches, xref_matches) for match in matches: if re.match(r"^\s+.*", match.group("value")) or re.match( From 583d1fdda6d202abbc5caa48bdb7bd07dc5d50a4 Mon Sep 17 00:00:00 2001 From: Christian Fischer Date: Thu, 12 Oct 2023 15:39:19 +0200 Subject: [PATCH 2/6] Change: Extend tests for script_tag_whitespaces.py plugin. --- tests/plugins/test_script_tag_whitespaces.py | 89 ++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/plugins/test_script_tag_whitespaces.py b/tests/plugins/test_script_tag_whitespaces.py index 4dae1bd4..ac6edb37 100644 --- a/tests/plugins/test_script_tag_whitespaces.py +++ b/tests/plugins/test_script_tag_whitespaces.py @@ -32,6 +32,10 @@ def test_ok(self): ' script_tag(name:"insight", value:"bar foo");\n' ' script_tag(name:"impact", value:"- foo\n - bar");\n' ' script_tag(name:"affected", value:"foo\n bar");\n' + ' script_xref(name:"foo1", value:"foo\nbar");\n' + ' script_xref(name:"foo2", value:"bar foo");\n' + ' script_xref(name:"foo3", value:"- foo\n - bar");\n' + ' script_xref(name:"foo4", value:"foo\n bar");\n' ) fake_context = self.create_file_plugin_context( nasl_file=self.path, file_content=content @@ -85,6 +89,23 @@ def test_script_name_leading_whitespace(self): results[0].message, ) + def test_script_xref_leading_whitespace(self): + content = ' script_xref(name:"foo", value:" bar");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptTagWhitespaces(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) + self.assertEqual( + 'script_xref(name:"foo", value:" bar");: value contains a' + " leading or trailing whitespace character", + results[0].message, + ) + def test_script_tag_trailing_whitespace(self): content = ' script_tag(name:"insight", value:"bar ");\n' fake_context = self.create_file_plugin_context( @@ -109,6 +130,23 @@ def test_script_name_trailing_whitespace(self): self.assertEqual(len(results), 1) self.assertIsInstance(results[0], LinterError) + def test_script_xref_trailing_whitespace(self): + content = ' script_xref(name:"foo", value:"bar ");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptTagWhitespaces(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) + self.assertEqual( + 'script_xref(name:"foo", value:"bar ");: value contains a' + " leading or trailing whitespace character", + results[0].message, + ) + # nb: The script_name() tag is not allowed to contain newlines (checked in a # dedicated plugin) so no specific test cases have been added here. def test_script_tag_trailing_newline(self): @@ -158,3 +196,54 @@ def test_script_tag_trailing_newline_with_newline_and_spaces(self): self.assertEqual(len(results), 1) self.assertIsInstance(results[0], LinterError) + + # nb: The value of script_xref(name:"URL" are also checked separately in + # script_xref_url() and that one would also report trailing / leading + # newlines and similar for that specific case + def test_script_xref_trailing_newline(self): + content = ' script_xref(name:"foo", value:"bar\n");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptTagWhitespaces(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) + + def test_script_xref_trailing_newline_with_space(self): + content = ' script_xref(name:"foo", value:"foo bar\n");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptTagWhitespaces(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) + + def test_script_xref_trailing_newline_with_newline(self): + content = ' script_xref(name:"foo", value:"foo\nbar\n");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptTagWhitespaces(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) + + def test_script_xref_trailing_newline_with_newline_and_spaces(self): + content = ' script_xref(name:"foo", value:"foo\n bar\n");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptTagWhitespaces(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) From 9ff8dbcc9d3e0e880aca79a4537b3d914c02eb40 Mon Sep 17 00:00:00 2001 From: Christian Fischer Date: Fri, 13 Oct 2023 06:09:37 +0200 Subject: [PATCH 3/6] Change: Drop duplicated _XREF_TAG_PATTERN. --- troubadix/helper/patterns.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/troubadix/helper/patterns.py b/troubadix/helper/patterns.py index fcb0226f..55752f42 100644 --- a/troubadix/helper/patterns.py +++ b/troubadix/helper/patterns.py @@ -178,12 +178,6 @@ def get_xref_pattern( r"(?P=quote99)?\s*\)\s*;" ) -_XREF_TAG_PATTERN = ( - r'script_xref\(\s*name\s*:\s*(?P[\'"])(?P{type})' - r'(?P=quote)\s*,\s*value\s*:\s*(?P[\'"])?(?P{' - r"value})(?P=quote2)?\s*\)\s*;" -) - class SpecialScriptTag(Enum): ADD_PREFERENCE = "add_preference" From 3d5e34f17bd66da0f02cbd003449d617f6a2e549 Mon Sep 17 00:00:00 2001 From: Christian Fischer Date: Fri, 13 Oct 2023 06:12:09 +0200 Subject: [PATCH 4/6] Change: Update get_xref_pattern/_XREF_TAG_PATTERN to work similar to _get_tag_pattern/TAG_PATTERN --- troubadix/helper/patterns.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/troubadix/helper/patterns.py b/troubadix/helper/patterns.py index 55752f42..946fe5fb 100644 --- a/troubadix/helper/patterns.py +++ b/troubadix/helper/patterns.py @@ -142,33 +142,30 @@ def get_script_tag_pattern(script_tag: ScriptTag) -> re.Pattern: _XREF_TAG_PATTERN = ( - r'script_xref\(\s*name\s*:\s*["\'](?P{type})["\']\s*,' - r'\s*value\s*:\s*["\']?(?P{value})["\']?\s*\)\s*;' + r'script_xref\(\s*name\s*:\s*(?P[\'"])(?P{name})(?P=quote)\s*,' + r'\s*value\s*:\s*(?P[\'"])?(?P{value})(?P=quote2)?\s*\)\s*;' ) def get_xref_pattern( - name: str, - *, - value: str = r".+", - flags: re.RegexFlag = 0, + name: str, *, value: str = r".+?", flags: re.RegexFlag = 0 ) -> re.Pattern: """ The returned pattern catches all - `script_xref(name="{type}", value="{value}");` + `script_xref(name="{name}", value="{value}");` Arguments: - name script xref type e.g. URL + name script xref name e.g. URL value script tag value (default: at least one char) flags regex flags for compile (default: 0) The returned `Match`s by this pattern will have group strings - .group('type') and .group('value') + .group('name') and .group('value') Returns `re.Pattern` object """ return re.compile( - _XREF_TAG_PATTERN.format(type=name, value=value), + _XREF_TAG_PATTERN.format(name=name, value=value), flags=flags, ) From b679054d42f8c68669e59e68325afdc01cfd1a78 Mon Sep 17 00:00:00 2001 From: Christian Fischer Date: Fri, 13 Oct 2023 06:22:03 +0200 Subject: [PATCH 5/6] Change: Minor reporting update. Adjust and extend tests. --- .../plugins/test_script_calls_empty_values.py | 3 ++- tests/plugins/test_script_xref_form.py | 26 ++++++++++++++++++- troubadix/plugins/script_xref_form.py | 2 +- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/plugins/test_script_calls_empty_values.py b/tests/plugins/test_script_calls_empty_values.py index ec3aa749..9e6e7f66 100644 --- a/tests/plugins/test_script_calls_empty_values.py +++ b/tests/plugins/test_script_calls_empty_values.py @@ -56,6 +56,7 @@ def test_missing_values(self): content = ( ' script_category("");\n' ' script_xref(name: "URL",value:"");\n' + ' script_xref(name:"URL", value:"");\n' ' script_tag(name:"", value:"");\n' ) fake_context = self.create_file_plugin_context( @@ -65,5 +66,5 @@ def test_missing_values(self): results = list(plugin.run()) - self.assertEqual(len(results), 2) + self.assertEqual(len(results), 3) self.assertIsInstance(results[0], LinterError) diff --git a/tests/plugins/test_script_xref_form.py b/tests/plugins/test_script_xref_form.py index 81213d55..b366be7c 100644 --- a/tests/plugins/test_script_xref_form.py +++ b/tests/plugins/test_script_xref_form.py @@ -58,7 +58,7 @@ def test_wrong_name(self): self.assertIsInstance(results[0], LinterError) self.assertEqual( 'script_xref(nammmme: "foo", value:"bar");: does not conform to' - ' script_xref(name:"", value:);', + ' script_xref(name:"", value:"");', results[0].message, ) @@ -85,3 +85,27 @@ def test_wrong_missing_parameters(self): self.assertEqual(len(results), 1) self.assertIsInstance(results[0], LinterError) + + def test_wrong_missing_name_parameter(self): + content = ' script_xref("foo", value:"bar");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptXrefForm(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) + + def test_wrong_missing_value_parameter(self): + content = ' script_xref(name:"foo", "bar");\n' + fake_context = self.create_file_plugin_context( + nasl_file=self.path, file_content=content + ) + plugin = CheckScriptXrefForm(fake_context) + + results = list(plugin.run()) + + self.assertEqual(len(results), 1) + self.assertIsInstance(results[0], LinterError) diff --git a/troubadix/plugins/script_xref_form.py b/troubadix/plugins/script_xref_form.py index e1bdcd52..fac88325 100644 --- a/troubadix/plugins/script_xref_form.py +++ b/troubadix/plugins/script_xref_form.py @@ -46,7 +46,7 @@ def check_content( ): yield LinterError( f"{match.group(0)}: does not conform to" - ' script_xref(name:"", value:);', + ' script_xref(name:"", value:"");', file=nasl_file, plugin=self.name, ) From 5f198971c66eb4c18df192f740a8989124aa156f Mon Sep 17 00:00:00 2001 From: mbrinkhoff Date: Thu, 9 Nov 2023 01:14:44 +0100 Subject: [PATCH 6/6] Bulletproof the XREF_TAG_PATTERN for overmatching --- troubadix/helper/patterns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/troubadix/helper/patterns.py b/troubadix/helper/patterns.py index 946fe5fb..10706d70 100644 --- a/troubadix/helper/patterns.py +++ b/troubadix/helper/patterns.py @@ -143,7 +143,7 @@ def get_script_tag_pattern(script_tag: ScriptTag) -> re.Pattern: _XREF_TAG_PATTERN = ( r'script_xref\(\s*name\s*:\s*(?P[\'"])(?P{name})(?P=quote)\s*,' - r'\s*value\s*:\s*(?P[\'"])?(?P{value})(?P=quote2)?\s*\)\s*;' + r'\s*value\s*:\s*(?P[\'"])(?P{value})(?P=quote2)\s*\)\s*;' )