Skip to content

Commit 5e3e694

Browse files
Daverballericwb
Daverball
and
ericwb
authored
Add markupsafe.Markup XSS plugin (#1225)
* Add markupsafe.Markup XSS plugin * Apply suggestions from code review Co-authored-by: Eric Brown <ericwb@users.noreply.github.com> --------- Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
1 parent 6133e08 commit 5e3e694

8 files changed

+216
-8
lines changed

‎bandit/core/context.py

+4
Original file line numberDiff line numberDiff line change
@@ -318,3 +318,7 @@ def filename(self):
318318
@property
319319
def file_data(self):
320320
return self._context.get("file_data")
321+
322+
@property
323+
def import_aliases(self):
324+
return self._context.get("import_aliases")

‎bandit/plugins/markupsafe_markup_xss.py

+118
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# Copyright (c) 2025 David Salvisberg
2+
#
3+
# SPDX-License-Identifier: Apache-2.0
4+
r"""
5+
============================================
6+
B704: Potential XSS on markupsafe.Markup use
7+
============================================
8+
9+
``markupsafe.Markup`` does not perform any escaping, so passing dynamic
10+
content, like f-strings, variables or interpolated strings will potentially
11+
lead to XSS vulnerabilities, especially if that data was submitted by users.
12+
13+
Instead you should interpolate the resulting ``markupsafe.Markup`` object,
14+
which will perform escaping, or use ``markupsafe.escape``.
15+
16+
17+
**Config Options:**
18+
19+
This plugin allows you to specify additional callable that should be treated
20+
like ``markupsafe.Markup``. By default we recognize ``flask.Markup`` as
21+
an alias, but there are other subclasses or similar classes in the wild
22+
that you may wish to treat the same.
23+
24+
Additionally there is a whitelist for callable names, whose result may
25+
be safely passed into ``markupsafe.Markup``. This is useful for escape
26+
functions like e.g. ``bleach.clean`` which don't themselves return
27+
``markupsafe.Markup``, so they need to be wrapped. Take care when using
28+
this setting, since incorrect use may introduce false negatives.
29+
30+
These two options can be set in a shared configuration section
31+
`markupsafe_xss`.
32+
33+
34+
.. code-block:: yaml
35+
36+
markupsafe_xss:
37+
# Recognize additional aliases
38+
extend_markup_names:
39+
- webhelpers.html.literal
40+
- my_package.Markup
41+
42+
# Allow the output of these functions to pass into Markup
43+
allowed_calls:
44+
- bleach.clean
45+
- my_package.sanitize
46+
47+
48+
:Example:
49+
50+
.. code-block:: none
51+
52+
>> Issue: [B704:markupsafe_markup_xss] Potential XSS with
53+
``markupsafe.Markup`` detected. Do not use ``Markup``
54+
on untrusted data.
55+
Severity: Medium Confidence: High
56+
CWE: CWE-79 (https://cwe.mitre.org/data/definitions/79.html)
57+
Location: ./examples/markupsafe_markup_xss.py:5:0
58+
4 content = "<script>alert('Hello, world!')</script>"
59+
5 Markup(f"unsafe {content}")
60+
6 flask.Markup("unsafe {}".format(content))
61+
62+
.. seealso::
63+
64+
- https://pypi.org/project/MarkupSafe/
65+
- https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup
66+
- https://cwe.mitre.org/data/definitions/79.html
67+
68+
.. versionadded:: 1.8.3
69+
70+
"""
71+
import ast
72+
73+
import bandit
74+
from bandit.core import issue
75+
from bandit.core import test_properties as test
76+
from bandit.core.utils import get_call_name
77+
78+
79+
def gen_config(name):
80+
if name == "markupsafe_xss":
81+
return {
82+
"extend_markup_names": [],
83+
"allowed_calls": [],
84+
}
85+
86+
87+
@test.takes_config("markupsafe_xss")
88+
@test.checks("Call")
89+
@test.test_id("B704")
90+
def markupsafe_markup_xss(context, config):
91+
92+
qualname = context.call_function_name_qual
93+
if qualname not in ("markupsafe.Markup", "flask.Markup"):
94+
if qualname not in config.get("extend_markup_names", []):
95+
# not a Markup call
96+
return None
97+
98+
args = context.node.args
99+
if not args or isinstance(args[0], ast.Constant):
100+
# both no arguments and a constant are fine
101+
return None
102+
103+
allowed_calls = config.get("allowed_calls", [])
104+
if (
105+
allowed_calls
106+
and isinstance(args[0], ast.Call)
107+
and get_call_name(args[0], context.import_aliases) in allowed_calls
108+
):
109+
# the argument contains a whitelisted call
110+
return None
111+
112+
return bandit.Issue(
113+
severity=bandit.MEDIUM,
114+
confidence=bandit.HIGH,
115+
cwe=issue.Cwe.XSS,
116+
text=f"Potential XSS with ``{qualname}`` detected. Do "
117+
f"not use ``{context.call_function_name}`` on untrusted data.",
118+
)

‎doc/source/plugins/b704_markupsafe_markup_xss.rst

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---------------------------
2+
B704: markupsafe_markup_xss
3+
---------------------------
4+
5+
.. automodule:: bandit.plugins.markupsafe_markup_xss

‎examples/markupsafe_markup_xss.py

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import flask
2+
from markupsafe import Markup, escape
3+
4+
content = "<script>alert('Hello, world!')</script>"
5+
Markup(f"unsafe {content}") # B704
6+
flask.Markup("unsafe {}".format(content)) # B704
7+
Markup("safe {}").format(content)
8+
flask.Markup(b"safe {}", encoding='utf-8').format(content)
9+
escape(content)
10+
Markup(content) # B704
11+
flask.Markup("unsafe %s" % content) # B704
12+
Markup(object="safe")
13+
Markup(object="unsafe {}".format(content)) # Not currently detected

‎examples/markupsafe_markup_xss_allowed_calls.py

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
from bleach import clean
2+
from markupsafe import Markup
3+
4+
content = "<script>alert('Hello, world!')</script>"
5+
Markup(clean(content))
6+
7+
# indirect assignments are currently not supported
8+
cleaned = clean(content)
9+
Markup(cleaned)

‎examples/markupsafe_markup_xss_extend_markup_names.py

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
from markupsafe import Markup
2+
from webhelpers.html import literal
3+
4+
content = "<script>alert('Hello, world!')</script>"
5+
Markup(f"unsafe {content}")
6+
literal(f"unsafe {content}")

‎setup.cfg

+3
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ bandit.plugins =
161161
# bandit/plugins/trojansource.py
162162
trojansource = bandit.plugins.trojansource:trojansource
163163

164+
# bandit/plugins/markupsafe_markup_xss.py
165+
markupsafe_markup_xss = bandit.plugins.markupsafe_markup_xss:markupsafe_markup_xss
166+
164167
[build_sphinx]
165168
all_files = 1
166169
build-dir = doc/build

‎tests/functional/test_functional.py

+58-8
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#
44
# SPDX-License-Identifier: Apache-2.0
55
import os
6+
from contextlib import contextmanager
67

78
import testtools
89

@@ -33,6 +34,18 @@ def setUp(self):
3334
self.b_mgr.b_conf._settings["plugins_dir"] = path
3435
self.b_mgr.b_ts = b_test_set.BanditTestSet(config=b_conf)
3536

37+
@contextmanager
38+
def with_test_set(self, ts):
39+
"""A helper context manager to change the test set without
40+
side-effects for any follow-up tests.
41+
"""
42+
orig_ts = self.b_mgr.b_ts
43+
self.b_mgr.b_ts = ts
44+
try:
45+
yield
46+
finally:
47+
self.b_mgr.b_ts = orig_ts
48+
3649
def run_example(self, example_script, ignore_nosec=False):
3750
"""A helper method to run the specified test
3851
@@ -526,21 +539,25 @@ def test_django_xss_secure(self):
526539
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
527540
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
528541
}
529-
self.b_mgr.b_ts = b_test_set.BanditTestSet(
530-
config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
531-
)
532-
self.check_example("mark_safe_secure.py", expect)
542+
with self.with_test_set(
543+
b_test_set.BanditTestSet(
544+
config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
545+
)
546+
):
547+
self.check_example("mark_safe_secure.py", expect)
533548

534549
def test_django_xss_insecure(self):
535550
"""Test for Django XSS via django.utils.safestring"""
536551
expect = {
537552
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 29, "HIGH": 0},
538553
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 29},
539554
}
540-
self.b_mgr.b_ts = b_test_set.BanditTestSet(
541-
config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
542-
)
543-
self.check_example("mark_safe_insecure.py", expect)
555+
with self.with_test_set(
556+
b_test_set.BanditTestSet(
557+
config=self.b_mgr.b_conf, profile={"exclude": ["B308"]}
558+
)
559+
):
560+
self.check_example("mark_safe_insecure.py", expect)
544561

545562
def test_xml(self):
546563
"""Test xml vulnerabilities."""
@@ -876,3 +893,36 @@ def test_trojansource_latin1(self):
876893
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
877894
}
878895
self.check_example("trojansource_latin1.py", expect)
896+
897+
def test_markupsafe_markup_xss(self):
898+
expect = {
899+
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 4, "HIGH": 0},
900+
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 4},
901+
}
902+
self.check_example("markupsafe_markup_xss.py", expect)
903+
904+
def test_markupsafe_markup_xss_extend_markup_names(self):
905+
expect = {
906+
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 2, "HIGH": 0},
907+
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 2},
908+
}
909+
b_conf = b_config.BanditConfig()
910+
b_conf.config["markupsafe_xss"] = {
911+
"extend_markup_names": ["webhelpers.html.literal"]
912+
}
913+
with self.with_test_set(b_test_set.BanditTestSet(config=b_conf)):
914+
self.check_example(
915+
"markupsafe_markup_xss_extend_markup_names.py", expect
916+
)
917+
918+
def test_markupsafe_markup_xss_allowed_calls(self):
919+
expect = {
920+
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 1, "HIGH": 0},
921+
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 1},
922+
}
923+
b_conf = b_config.BanditConfig()
924+
b_conf.config["markupsafe_xss"] = {"allowed_calls": ["bleach.clean"]}
925+
with self.with_test_set(b_test_set.BanditTestSet(config=b_conf)):
926+
self.check_example(
927+
"markupsafe_markup_xss_allowed_calls.py", expect
928+
)

0 commit comments

Comments
 (0)