From 4da6936ec48258a9d425068685fd8b12a7fff2ee Mon Sep 17 00:00:00 2001 From: Mauro Fontana Date: Mon, 17 Mar 2025 11:29:07 +0100 Subject: [PATCH] [`flake8-bandit`] Allow raw strings in `suspicious-mark-safe-usage` (`S308`) #16702 (#16770) ## Summary Stop flagging each invocation of `django.utils.safestring.mark_safe` (also available at, `django.utils.html.mark_safe`) as an error. Instead, allow string literals as valid uses for `mark_safe`. Also, update the documentation, pointing at `django.utils.html.format_html` for dynamic content generation use cases. Closes #16702 ## Test Plan I verified several possible uses, but string literals, are still flagged. --------- Co-authored-by: Micha Reiser --- .../test/fixtures/flake8_bandit/S308.py | 24 +++- .../rules/suspicious_function_call.rs | 27 +++- ...s__flake8_bandit__tests__S308_S308.py.snap | 119 +++++++++++++--- ..._bandit__tests__preview__S308_S308.py.snap | 134 ++++++++++++++---- 4 files changed, 253 insertions(+), 51 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S308.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S308.py index 46314be5d5..c952036b1c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S308.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S308.py @@ -1,8 +1,16 @@ from django.utils.safestring import mark_safe -def some_func(): - return mark_safe('') +def bad_func(): + inject = "harmful_input" + mark_safe(inject) + mark_safe("I will add" + inject + "to my string") + mark_safe("I will add %s to my string" % inject) + mark_safe("I will add {} to my string".format(inject)) + mark_safe(f"I will add {inject} to my string") + +def good_func(): + mark_safe("I won't inject anything") @mark_safe @@ -13,8 +21,16 @@ def some_func(): from django.utils.html import mark_safe -def some_func(): - return mark_safe('') +def bad_func(): + inject = "harmful_input" + mark_safe(inject) + mark_safe("I will add" + inject + "to my string") + mark_safe("I will add %s to my string" % inject) + mark_safe("I will add {} to my string".format(inject)) + mark_safe(f"I will add {inject} to my string") + +def good_func(): + mark_safe("I won't inject anything") @mark_safe diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs index 08b20a1fdf..f47ecc06f4 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs @@ -344,21 +344,31 @@ impl Violation for SuspiciousEvalUsage { /// before rending them. /// /// `django.utils.safestring.mark_safe` marks a string as safe for use in HTML -/// templates, bypassing XSS protection. This is dangerous because it may allow +/// templates, bypassing XSS protection. Its usage can be dangerous if the +/// contents of the string are dynamically generated, because it may allow /// cross-site scripting attacks if the string is not properly escaped. /// +/// For dynamically generated strings, consider utilizing +/// `django.utils.html.format_html`. +/// /// In [preview], this rule will also flag references to `django.utils.safestring.mark_safe`. /// /// ## Example /// ```python /// from django.utils.safestring import mark_safe /// -/// content = mark_safe("") # XSS. +/// +/// def render_username(username): +/// return mark_safe(f"{username}") # Dangerous if username is user-provided. /// ``` /// /// Use instead: /// ```python -/// content = "" # Safe if rendered. +/// from django.utils.html import format_html +/// +/// +/// def render_username(username): +/// return django.utils.html.format_html("{}", username) # username is escaped. /// ``` /// /// ## References @@ -1059,7 +1069,16 @@ fn suspicious_function( ["" | "builtins", "eval"] => SuspiciousEvalUsage.into(), // MarkSafe - ["django", "utils", "safestring" | "html", "mark_safe"] => SuspiciousMarkSafeUsage.into(), + ["django", "utils", "safestring" | "html", "mark_safe"] => { + if let Some(arguments) = arguments { + if let [single] = &*arguments.args { + if single.is_string_literal_expr() { + return; + } + } + } + SuspiciousMarkSafeUsage.into() + } // URLOpen (`Request`) ["urllib", "request", "Request"] | ["six", "moves", "urllib", "request", "Request"] => { diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S308_S308.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S308_S308.py.snap index b5b6b00038..f48701e74c 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S308_S308.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S308_S308.py.snap @@ -1,33 +1,116 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs -snapshot_kind: text --- -S308.py:5:12: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:6:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -4 | def some_func(): -5 | return mark_safe('') - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +4 | def bad_func(): +5 | inject = "harmful_input" +6 | mark_safe(inject) + | ^^^^^^^^^^^^^^^^^ S308 +7 | mark_safe("I will add" + inject + "to my string") +8 | mark_safe("I will add %s to my string" % inject) | -S308.py:8:1: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:7:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +5 | inject = "harmful_input" +6 | mark_safe(inject) +7 | mark_safe("I will add" + inject + "to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +8 | mark_safe("I will add %s to my string" % inject) +9 | mark_safe("I will add {} to my string".format(inject)) + | + +S308.py:8:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | - 8 | @mark_safe - | ^^^^^^^^^^ S308 - 9 | def some_func(): -10 | return '' + 6 | mark_safe(inject) + 7 | mark_safe("I will add" + inject + "to my string") + 8 | mark_safe("I will add %s to my string" % inject) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 + 9 | mark_safe("I will add {} to my string".format(inject)) +10 | mark_safe(f"I will add {inject} to my string") | -S308.py:17:12: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:9:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -16 | def some_func(): -17 | return mark_safe('') - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 + 7 | mark_safe("I will add" + inject + "to my string") + 8 | mark_safe("I will add %s to my string" % inject) + 9 | mark_safe("I will add {} to my string".format(inject)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +10 | mark_safe(f"I will add {inject} to my string") | -S308.py:20:1: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:10:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -20 | @mark_safe + 8 | mark_safe("I will add %s to my string" % inject) + 9 | mark_safe("I will add {} to my string".format(inject)) +10 | mark_safe(f"I will add {inject} to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +11 | +12 | def good_func(): + | + +S308.py:16:1: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +16 | @mark_safe | ^^^^^^^^^^ S308 -21 | def some_func(): -22 | return '' +17 | def some_func(): +18 | return '' + | + +S308.py:26:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +24 | def bad_func(): +25 | inject = "harmful_input" +26 | mark_safe(inject) + | ^^^^^^^^^^^^^^^^^ S308 +27 | mark_safe("I will add" + inject + "to my string") +28 | mark_safe("I will add %s to my string" % inject) + | + +S308.py:27:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +25 | inject = "harmful_input" +26 | mark_safe(inject) +27 | mark_safe("I will add" + inject + "to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +28 | mark_safe("I will add %s to my string" % inject) +29 | mark_safe("I will add {} to my string".format(inject)) + | + +S308.py:28:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +26 | mark_safe(inject) +27 | mark_safe("I will add" + inject + "to my string") +28 | mark_safe("I will add %s to my string" % inject) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +29 | mark_safe("I will add {} to my string".format(inject)) +30 | mark_safe(f"I will add {inject} to my string") + | + +S308.py:29:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +27 | mark_safe("I will add" + inject + "to my string") +28 | mark_safe("I will add %s to my string" % inject) +29 | mark_safe("I will add {} to my string".format(inject)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +30 | mark_safe(f"I will add {inject} to my string") + | + +S308.py:30:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +28 | mark_safe("I will add %s to my string" % inject) +29 | mark_safe("I will add {} to my string".format(inject)) +30 | mark_safe(f"I will add {inject} to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +31 | +32 | def good_func(): + | + +S308.py:36:1: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +36 | @mark_safe + | ^^^^^^^^^^ S308 +37 | def some_func(): +38 | return '' | diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S308_S308.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S308_S308.py.snap index fc5bd98fe9..bc0f4d524e 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S308_S308.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S308_S308.py.snap @@ -1,48 +1,132 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S308.py:5:12: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:6:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -4 | def some_func(): -5 | return mark_safe('') - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +4 | def bad_func(): +5 | inject = "harmful_input" +6 | mark_safe(inject) + | ^^^^^^^^^^^^^^^^^ S308 +7 | mark_safe("I will add" + inject + "to my string") +8 | mark_safe("I will add %s to my string" % inject) | -S308.py:8:2: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:7:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +5 | inject = "harmful_input" +6 | mark_safe(inject) +7 | mark_safe("I will add" + inject + "to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +8 | mark_safe("I will add %s to my string" % inject) +9 | mark_safe("I will add {} to my string".format(inject)) + | + +S308.py:8:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | - 8 | @mark_safe - | ^^^^^^^^^ S308 - 9 | def some_func(): -10 | return '' + 6 | mark_safe(inject) + 7 | mark_safe("I will add" + inject + "to my string") + 8 | mark_safe("I will add %s to my string" % inject) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 + 9 | mark_safe("I will add {} to my string".format(inject)) +10 | mark_safe(f"I will add {inject} to my string") | -S308.py:17:12: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:9:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -16 | def some_func(): -17 | return mark_safe('') - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 + 7 | mark_safe("I will add" + inject + "to my string") + 8 | mark_safe("I will add %s to my string" % inject) + 9 | mark_safe("I will add {} to my string".format(inject)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +10 | mark_safe(f"I will add {inject} to my string") | -S308.py:20:2: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:10:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -20 | @mark_safe + 8 | mark_safe("I will add %s to my string" % inject) + 9 | mark_safe("I will add {} to my string".format(inject)) +10 | mark_safe(f"I will add {inject} to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +11 | +12 | def good_func(): + | + +S308.py:16:2: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +16 | @mark_safe | ^^^^^^^^^ S308 -21 | def some_func(): -22 | return '' +17 | def some_func(): +18 | return '' | S308.py:26:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -25 | # https://github.com/astral-sh/ruff/issues/15522 -26 | map(mark_safe, []) - | ^^^^^^^^^ S308 -27 | foo = mark_safe +24 | def bad_func(): +25 | inject = "harmful_input" +26 | mark_safe(inject) + | ^^^^^^^^^^^^^^^^^ S308 +27 | mark_safe("I will add" + inject + "to my string") +28 | mark_safe("I will add %s to my string" % inject) | -S308.py:27:7: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities +S308.py:27:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities | -25 | # https://github.com/astral-sh/ruff/issues/15522 -26 | map(mark_safe, []) -27 | foo = mark_safe +25 | inject = "harmful_input" +26 | mark_safe(inject) +27 | mark_safe("I will add" + inject + "to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +28 | mark_safe("I will add %s to my string" % inject) +29 | mark_safe("I will add {} to my string".format(inject)) + | + +S308.py:28:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +26 | mark_safe(inject) +27 | mark_safe("I will add" + inject + "to my string") +28 | mark_safe("I will add %s to my string" % inject) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +29 | mark_safe("I will add {} to my string".format(inject)) +30 | mark_safe(f"I will add {inject} to my string") + | + +S308.py:29:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +27 | mark_safe("I will add" + inject + "to my string") +28 | mark_safe("I will add %s to my string" % inject) +29 | mark_safe("I will add {} to my string".format(inject)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +30 | mark_safe(f"I will add {inject} to my string") + | + +S308.py:30:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +28 | mark_safe("I will add %s to my string" % inject) +29 | mark_safe("I will add {} to my string".format(inject)) +30 | mark_safe(f"I will add {inject} to my string") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S308 +31 | +32 | def good_func(): + | + +S308.py:36:2: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +36 | @mark_safe + | ^^^^^^^^^ S308 +37 | def some_func(): +38 | return '' + | + +S308.py:42:5: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +41 | # https://github.com/astral-sh/ruff/issues/15522 +42 | map(mark_safe, []) + | ^^^^^^^^^ S308 +43 | foo = mark_safe + | + +S308.py:43:7: S308 Use of `mark_safe` may expose cross-site scripting vulnerabilities + | +41 | # https://github.com/astral-sh/ruff/issues/15522 +42 | map(mark_safe, []) +43 | foo = mark_safe | ^^^^^^^^^ S308 |