Repository: PyCQA/bandit
Pull request #1230 (2025-02-04 23:33:33)

Warn not error on an nonexistant test given
When a user gives a test ID to include or skip, the current behavior raises an exception and exits the process.

However, when tests end up getting deprecated and eventually removed, it is a lot more user friendly to simple present a warning to the user that the test ID given wasn't found rather than a hard error and exit.

Fixes: #1228, #1226


Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug-report.yml b/.github/ISSUE_TEMPLATE/bug-report.yml
index 4569f95..c968cc4 100644
--- a/.github/ISSUE_TEMPLATE/bug-report.yml
+++ b/.github/ISSUE_TEMPLATE/bug-report.yml
@@ -44,7 +44,9 @@ body:
       label: Bandit version
       description: Run "bandit --version" if unsure of version number
       options:
-        - 1.8.0 (Default)
+        - 1.8.2 (Default)
+        - 1.8.1
+        - 1.8.0
         - 1.7.10
         - 1.7.9
         - 1.7.8
diff --git a/bandit/core/context.py b/bandit/core/context.py
index 8a2d4fb..57f293c 100644
--- a/bandit/core/context.py
+++ b/bandit/core/context.py
@@ -318,3 +318,7 @@ class Context:
     @property
     def file_data(self):
         return self._context.get("file_data")
+
+    @property
+    def import_aliases(self):
+        return self._context.get("import_aliases")
diff --git a/bandit/core/extension_loader.py b/bandit/core/extension_loader.py
index 05fd9fb..ec28a0a 100644
--- a/bandit/core/extension_loader.py
+++ b/bandit/core/extension_loader.py
@@ -1,11 +1,14 @@
 #
 # SPDX-License-Identifier: Apache-2.0
+import logging
 import sys
 
 from stevedore import extension
 
 from bandit.core import utils
 
+LOG = logging.getLogger(__name__)
+
 
 class Manager:
     # These IDs are for bandit built in tests
@@ -84,11 +87,11 @@ class Manager:
         """Validate that everything in the configured profiles looks good."""
         for inc in profile["include"]:
             if not self.check_id(inc):
-                raise ValueError(f"Unknown test found in profile: {inc}")
+                LOG.warning(f"Unknown test found in profile: {inc}")
 
         for exc in profile["exclude"]:
             if not self.check_id(exc):
-                raise ValueError(f"Unknown test found in profile: {exc}")
+                LOG.warning(f"Unknown test found in profile: {exc}")
 
         union = set(profile["include"]) & set(profile["exclude"])
         if len(union) > 0:
diff --git a/bandit/plugins/markupsafe_markup_xss.py b/bandit/plugins/markupsafe_markup_xss.py
new file mode 100644
index 0000000..7eae905
--- /dev/null
+++ b/bandit/plugins/markupsafe_markup_xss.py
@@ -0,0 +1,118 @@
+# Copyright (c) 2025 David Salvisberg
+#
+# SPDX-License-Identifier: Apache-2.0
+r"""
+============================================
+B704: Potential XSS on markupsafe.Markup use
+============================================
+
+``markupsafe.Markup`` does not perform any escaping, so passing dynamic
+content, like f-strings, variables or interpolated strings will potentially
+lead to XSS vulnerabilities, especially if that data was submitted by users.
+
+Instead you should interpolate the resulting ``markupsafe.Markup`` object,
+which will perform escaping, or use ``markupsafe.escape``.
+
+
+**Config Options:**
+
+This plugin allows you to specify additional callable that should be treated
+like ``markupsafe.Markup``. By default we recognize ``flask.Markup`` as
+an alias, but there are other subclasses or similar classes in the wild
+that you may wish to treat the same.
+
+Additionally there is a whitelist for callable names, whose result may
+be safely passed into ``markupsafe.Markup``. This is useful for escape
+functions like e.g. ``bleach.clean`` which don't themselves return
+``markupsafe.Markup``, so they need to be wrapped. Take care when using
+this setting, since incorrect use may introduce false negatives.
+
+These two options can be set in a shared configuration section
+`markupsafe_xss`.
+
+
+.. code-block:: yaml
+
+    markupsafe_xss:
+        # Recognize additional aliases
+        extend_markup_names:
+            - webhelpers.html.literal
+            - my_package.Markup
+
+        # Allow the output of these functions to pass into Markup
+        allowed_calls:
+            - bleach.clean
+            - my_package.sanitize
+
+
+:Example:
+
+.. code-block:: none
+
+    >> Issue: [B704:markupsafe_markup_xss] Potential XSS with
+       ``markupsafe.Markup`` detected. Do not use ``Markup``
+       on untrusted data.
+       Severity: Medium   Confidence: High
+       CWE: CWE-79 (https://cwe.mitre.org/data/definitions/79.html)
+       Location: ./examples/markupsafe_markup_xss.py:5:0
+    4       content = "<script>alert('Hello, world!')</script>"
+    5       Markup(f"unsafe {content}")
+    6       flask.Markup("unsafe {}".format(content))
+
+.. seealso::
+
+ - https://pypi.org/project/MarkupSafe/
+ - https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup
+ - https://cwe.mitre.org/data/definitions/79.html
+
+.. versionadded:: 1.8.3
+
+"""
+import ast
+
+import bandit
+from bandit.core import issue
+from bandit.core import test_properties as test
+from bandit.core.utils import get_call_name
+
+
+def gen_config(name):
+    if name == "markupsafe_xss":
+        return {
+            "extend_markup_names": [],
+            "allowed_calls": [],
+        }
+
+
+@test.takes_config("markupsafe_xss")
+@test.checks("Call")
+@test.test_id("B704")
+def markupsafe_markup_xss(context, config):
+
+    qualname = context.call_function_name_qual
+    if qualname not in ("markupsafe.Markup", "flask.Markup"):
+        if qualname not in config.get("extend_markup_names", []):
+            # not a Markup call
+            return None
+
+    args = context.node.args
+    if not args or isinstance(args[0], ast.Constant):
+        # both no arguments and a constant are fine
+        return None
+
+    allowed_calls = config.get("allowed_calls", [])
+    if (
+        allowed_calls
+        and isinstance(args[0], ast.Call)
+        and get_call_name(args[0], context.import_aliases) in allowed_calls
+    ):
+        # the argument contains a whitelisted call
+        return None
+
+    return bandit.Issue(
+        severity=bandit.MEDIUM,
+        confidence=bandit.HIGH,
+        cwe=issue.Cwe.XSS,
+        text=f"Potential XSS with ``{qualname}`` detected. Do "
+        f"not use ``{context.call_function_name}`` on untrusted data.",
+    )
diff --git a/doc/source/plugins/b704_markupsafe_markup_xss.rst b/doc/source/plugins/b704_markupsafe_markup_xss.rst
new file mode 100644
index 0000000..47eb699
--- /dev/null
+++ b/doc/source/plugins/b704_markupsafe_markup_xss.rst
@@ -0,0 +1,5 @@
+---------------------------
+B704: markupsafe_markup_xss
+---------------------------
+
+.. automodule:: bandit.plugins.markupsafe_markup_xss
diff --git a/examples/markupsafe_markup_xss.py b/examples/markupsafe_markup_xss.py
new file mode 100644
index 0000000..23ee147
--- /dev/null
+++ b/examples/markupsafe_markup_xss.py
@@ -0,0 +1,13 @@
+import flask
+from markupsafe import Markup, escape
+
+content = "<script>alert('Hello, world!')</script>"
+Markup(f"unsafe {content}")  # B704
+flask.Markup("unsafe {}".format(content))  # B704
+Markup("safe {}").format(content)
+flask.Markup(b"safe {}", encoding='utf-8').format(content)
+escape(content)
+Markup(content)  # B704
+flask.Markup("unsafe %s" % content)  # B704
+Markup(object="safe")
+Markup(object="unsafe {}".format(content))  # Not currently detected
diff --git a/examples/markupsafe_markup_xss_allowed_calls.py b/examples/markupsafe_markup_xss_allowed_calls.py
new file mode 100644
index 0000000..95519ec
--- /dev/null
+++ b/examples/markupsafe_markup_xss_allowed_calls.py
@@ -0,0 +1,9 @@
+from bleach import clean
+from markupsafe import Markup
+
+content = "<script>alert('Hello, world!')</script>"
+Markup(clean(content))
+
+# indirect assignments are currently not supported
+cleaned = clean(content)
+Markup(cleaned)
diff --git a/examples/markupsafe_markup_xss_extend_markup_names.py b/examples/markupsafe_markup_xss_extend_markup_names.py
new file mode 100644
index 0000000..2ae44c0
--- /dev/null
+++ b/examples/markupsafe_markup_xss_extend_markup_names.py
@@ -0,0 +1,6 @@
+from markupsafe import Markup
+from webhelpers.html import literal
+
+content = "<script>alert('Hello, world!')</script>"
+Markup(f"unsafe {content}")
+literal(f"unsafe {content}")
diff --git a/setup.cfg b/setup.cfg
index e7868e5..83d57d1 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -161,6 +161,9 @@ bandit.plugins =
     # bandit/plugins/trojansource.py
     trojansource = bandit.plugins.trojansource:trojansource
 
+    # bandit/plugins/markupsafe_markup_xss.py
+    markupsafe_markup_xss = bandit.plugins.markupsafe_markup_xss:markupsafe_markup_xss
+
 [build_sphinx]
 all_files = 1
 build-dir = doc/build


Test patch:
diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py
index f9943f7..f9fe695 100644
--- a/tests/functional/test_functional.py
+++ b/tests/functional/test_functional.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 import os
+from contextlib import contextmanager
 
 import testtools
 
@@ -33,6 +34,18 @@ class FunctionalTests(testtools.TestCase):
         self.b_mgr.b_conf._settings["plugins_dir"] = path
         self.b_mgr.b_ts = b_test_set.BanditTestSet(config=b_conf)
 
+    @contextmanager
+    def with_test_set(self, ts):
+        """A helper context manager to change the test set without
+        side-effects for any follow-up tests.
+        """
+        orig_ts = self.b_mgr.b_ts
+        self.b_mgr.b_ts = ts
+        try:
+            yield
+        finally:
+            self.b_mgr.b_ts = orig_ts
+
     def run_example(self, example_script, ignore_nosec=False):
         """A helper method to run the specified test
 
@@ -526,10 +539,12 @@ class FunctionalTests(testtools.TestCase):
             "SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
             "CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
         }
-        self.b_mgr.b_ts = b_test_set.BanditTestSet(
-            config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
-        )
-        self.check_example("mark_safe_secure.py", expect)
+        with self.with_test_set(
+            b_test_set.BanditTestSet(
+                config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
+            )
+        ):
+            self.check_example("mark_safe_secure.py", expect)
 
     def test_django_xss_insecure(self):
         """Test for Django XSS via django.utils.safestring"""
@@ -537,10 +552,12 @@ class FunctionalTests(testtools.TestCase):
             "SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 29, "HIGH": 0},
             "CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 29},
         }
-        self.b_mgr.b_ts = b_test_set.BanditTestSet(
-            config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
-        )
-        self.check_example("mark_safe_insecure.py", expect)
+        with self.with_test_set(
+            b_test_set.BanditTestSet(
+                config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
+            )
+        ):
+            self.check_example("mark_safe_insecure.py", expect)
 
     def test_xml(self):
         """Test xml vulnerabilities."""
@@ -876,3 +893,36 @@ class FunctionalTests(testtools.TestCase):
             "CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
         }
         self.check_example("trojansource_latin1.py", expect)
+
+    def test_markupsafe_markup_xss(self):
+        expect = {
+            "SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 4, "HIGH": 0},
+            "CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 4},
+        }
+        self.check_example("markupsafe_markup_xss.py", expect)
+
+    def test_markupsafe_markup_xss_extend_markup_names(self):
+        expect = {
+            "SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 2, "HIGH": 0},
+            "CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 2},
+        }
+        b_conf = b_config.BanditConfig()
+        b_conf.config["markupsafe_xss"] = {
+            "extend_markup_names": ["webhelpers.html.literal"]
+        }
+        with self.with_test_set(b_test_set.BanditTestSet(config=b_conf)):
+            self.check_example(
+                "markupsafe_markup_xss_extend_markup_names.py", expect
+            )
+
+    def test_markupsafe_markup_xss_allowed_calls(self):
+        expect = {
+            "SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 1, "HIGH": 0},
+            "CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 1},
+        }
+        b_conf = b_config.BanditConfig()
+        b_conf.config["markupsafe_xss"] = {"allowed_calls": ["bleach.clean"]}
+        with self.with_test_set(b_test_set.BanditTestSet(config=b_conf)):
+            self.check_example(
+                "markupsafe_markup_xss_allowed_calls.py", expect
+            )
diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py
index 0852db7..98b95ec 100644
--- a/tests/unit/cli/test_main.py
+++ b/tests/unit/cli/test_main.py
@@ -215,33 +215,9 @@ class BanditCLIMainTests(testtools.TestCase):
                 self.assertRaisesRegex(SystemExit, "2", bandit.main)
                 self.assertEqual(
                     str(err_mock.call_args[0][0]),
-                    "Unknown test found in profile: some_test",
+                    "No tests would be run, please check the profile.",
                 )
 
-    @mock.patch(
-        "sys.argv", ["bandit", "-c", "bandit.yaml", "-t", "badID", "test"]
-    )
-    def test_main_unknown_tests(self):
-        # Test that bandit exits when an invalid test ID is provided
-        temp_directory = self.useFixture(fixtures.TempDir()).path
-        os.chdir(temp_directory)
-        with open("bandit.yaml", "w") as fd:
-            fd.write(bandit_config_content)
-        # assert a SystemExit with code 2
-        self.assertRaisesRegex(SystemExit, "2", bandit.main)
-
-    @mock.patch(
-        "sys.argv", ["bandit", "-c", "bandit.yaml", "-s", "badID", "test"]
-    )
-    def test_main_unknown_skip_tests(self):
-        # Test that bandit exits when an invalid test ID is provided to skip
-        temp_directory = self.useFixture(fixtures.TempDir()).path
-        os.chdir(temp_directory)
-        with open("bandit.yaml", "w") as fd:
-            fd.write(bandit_config_content)
-        # assert a SystemExit with code 2
-        self.assertRaisesRegex(SystemExit, "2", bandit.main)
-
     @mock.patch(
         "sys.argv", ["bandit", "-c", "bandit.yaml", "-p", "bad", "test"]
     )