From c1f4a68cb6b94f00326bf978c0f8f8a0e067be91 Mon Sep 17 00:00:00 2001 From: Chaojie Date: Tue, 7 Nov 2023 11:16:50 +0800 Subject: [PATCH 1/2] [flake8-bandit] Add Rule for S702 (use of mako templates) --- .../test/fixtures/flake8_bandit/S702.py | 11 ++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_bandit/mod.rs | 1 + .../flake8_bandit/rules/mako_templates.rs | 59 +++++++++++++++++++ .../src/rules/flake8_bandit/rules/mod.rs | 2 + ...s__flake8_bandit__tests__S702_S702.py.snap | 31 ++++++++++ ruff.schema.json | 1 + 8 files changed, 109 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py new file mode 100644 index 00000000000000..29dd38c9f7ebc0 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py @@ -0,0 +1,11 @@ +from mako.template import Template +import mako + +from mako import template + +Template("hello") + +# XXX(fletcher): for some reason, bandit is missing the one below. keeping it +# in for now so that if it gets fixed inadvertitently we know. +mako.template.Template("hern") +template.Template("hern") diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index e769f40e3cd7ec..106cbecd47909e 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -571,6 +571,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::Jinja2AutoescapeFalse) { flake8_bandit::rules::jinja2_autoescape_false(checker, call); } + if checker.enabled(Rule::MakoTemplates) { + flake8_bandit::rules::mako_templates(checker, call); + } if checker.enabled(Rule::HardcodedPasswordFuncArg) { flake8_bandit::rules::hardcoded_password_func_arg(checker, keywords); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 320b62020d4b31..b5ae7108fc64c4 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -629,6 +629,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bandit, "609") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnixCommandWildcardInjection), (Flake8Bandit, "612") => (RuleGroup::Stable, rules::flake8_bandit::rules::LoggingConfigInsecureListen), (Flake8Bandit, "701") => (RuleGroup::Stable, rules::flake8_bandit::rules::Jinja2AutoescapeFalse), + (Flake8Bandit, "702") => (RuleGroup::Stable, rules::flake8_bandit::rules::MakoTemplates), // flake8-boolean-trap (Flake8BooleanTrap, "001") => (RuleGroup::Stable, rules::flake8_boolean_trap::rules::BooleanTypeHintPositionalArgument), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index f915fd88190ad5..3ad7a8e421b415 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs @@ -28,6 +28,7 @@ mod tests { #[test_case(Rule::HardcodedTempFile, Path::new("S108.py"))] #[test_case(Rule::HashlibInsecureHashFunction, Path::new("S324.py"))] #[test_case(Rule::Jinja2AutoescapeFalse, Path::new("S701.py"))] + #[test_case(Rule::MakoTemplates, Path::new("S702.py"))] #[test_case(Rule::LoggingConfigInsecureListen, Path::new("S612.py"))] #[test_case(Rule::ParamikoCall, Path::new("S601.py"))] #[test_case(Rule::RequestWithNoCertValidation, Path::new("S501.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs new file mode 100644 index 00000000000000..8981df62c4efe0 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs @@ -0,0 +1,59 @@ +use crate::checkers::ast::Checker; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast}; +use ruff_text_size::Ranged; + +/// ## What it does +/// Checks for uses of the `mako` templates. +/// +/// ## Why is this bad? +/// Mako templates allow HTML/JS rendering by default and are +/// inherently open to XSS attacks. Ensure variables in all templates are +/// properly sanitized via the 'n', 'h' or 'x' flags (depending on context). +/// For example, to HTML escape the variable 'data' do ${ data |h }. +/// +/// ## Example +/// ```python +/// from mako.template import Template +/// +/// Template("hello") +/// ``` +/// +/// Use instead: +/// ```python +/// from mako.template import Template +/// +/// Template("hello |h") +/// ``` +/// +/// ## References +/// - [Mako documentation](https://www.makotemplates.org/) +/// - [OpenStack security: Cross site scripting XSS](https://security.openstack.org/guidelines/dg_cross-site-scripting-xss.html) +/// - [Common Weakness Enumeration: CWE-80](https://cwe.mitre.org/data/definitions/80.html) +#[violation] +pub struct MakoTemplates; + +impl Violation for MakoTemplates { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.\ + Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).\ + For example, to HTML escape the variable 'data' do ${{ data |h }}." + ) + } +} + +/// S702 +pub(crate) fn mako_templates(checker: &mut Checker, call: &ast::ExprCall) { + if checker + .semantic() + .resolve_call_path(&call.func) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["mako", "template", "Template"])) + { + checker + .diagnostics + .push(Diagnostic::new(MakoTemplates, call.func.range())); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs index 24ad5a3721c856..b1ebad53e41caa 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs @@ -11,6 +11,7 @@ pub(crate) use hardcoded_tmp_directory::*; pub(crate) use hashlib_insecure_hash_functions::*; pub(crate) use jinja2_autoescape_false::*; pub(crate) use logging_config_insecure_listen::*; +pub(crate) use mako_templates::*; pub(crate) use paramiko_calls::*; pub(crate) use request_with_no_cert_validation::*; pub(crate) use request_without_timeout::*; @@ -37,6 +38,7 @@ mod hardcoded_tmp_directory; mod hashlib_insecure_hash_functions; mod jinja2_autoescape_false; mod logging_config_insecure_listen; +mod mako_templates; mod paramiko_calls; mod request_with_no_cert_validation; mod request_without_timeout; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap new file mode 100644 index 00000000000000..8b504138b4cecc --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap @@ -0,0 +1,31 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs +--- +S702.py:6:1: S702 Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).For example, to HTML escape the variable 'data' do ${ data |h }. + | +4 | from mako import template +5 | +6 | Template("hello") + | ^^^^^^^^ S702 +7 | +8 | # XXX(fletcher): for some reason, bandit is missing the one below. keeping it + | + +S702.py:10:1: S702 Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).For example, to HTML escape the variable 'data' do ${ data |h }. + | + 8 | # XXX(fletcher): for some reason, bandit is missing the one below. keeping it + 9 | # in for now so that if it gets fixed inadvertitently we know. +10 | mako.template.Template("hern") + | ^^^^^^^^^^^^^^^^^^^^^^ S702 +11 | template.Template("hern") + | + +S702.py:11:1: S702 Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).For example, to HTML escape the variable 'data' do ${ data |h }. + | + 9 | # in for now so that if it gets fixed inadvertitently we know. +10 | mako.template.Template("hern") +11 | template.Template("hern") + | ^^^^^^^^^^^^^^^^^ S702 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index c77e480c468605..30f4b342889def 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3386,6 +3386,7 @@ "S7", "S70", "S701", + "S702", "SIM", "SIM1", "SIM10", From ce1329dd6b43e14f3f62b74b53c19b5b24bbaf66 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 7 Nov 2023 15:53:53 -0500 Subject: [PATCH 2/2] Tweaks --- .../test/fixtures/flake8_bandit/S702.py | 4 +-- crates/ruff_linter/src/codes.rs | 2 +- .../flake8_bandit/rules/mako_templates.rs | 10 +++--- ...s__flake8_bandit__tests__S702_S702.py.snap | 35 +++++++++---------- 4 files changed, 22 insertions(+), 29 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py index 29dd38c9f7ebc0..b1e11fc0077ccb 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S702.py @@ -1,11 +1,9 @@ from mako.template import Template +from mako import template import mako -from mako import template Template("hello") -# XXX(fletcher): for some reason, bandit is missing the one below. keeping it -# in for now so that if it gets fixed inadvertitently we know. mako.template.Template("hern") template.Template("hern") diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index b5ae7108fc64c4..5f84914aaee352 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -629,7 +629,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bandit, "609") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnixCommandWildcardInjection), (Flake8Bandit, "612") => (RuleGroup::Stable, rules::flake8_bandit::rules::LoggingConfigInsecureListen), (Flake8Bandit, "701") => (RuleGroup::Stable, rules::flake8_bandit::rules::Jinja2AutoescapeFalse), - (Flake8Bandit, "702") => (RuleGroup::Stable, rules::flake8_bandit::rules::MakoTemplates), + (Flake8Bandit, "702") => (RuleGroup::Preview, rules::flake8_bandit::rules::MakoTemplates), // flake8-boolean-trap (Flake8BooleanTrap, "001") => (RuleGroup::Stable, rules::flake8_boolean_trap::rules::BooleanTypeHintPositionalArgument), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs index 8981df62c4efe0..b101792b229fb9 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/mako_templates.rs @@ -8,10 +8,10 @@ use ruff_text_size::Ranged; /// Checks for uses of the `mako` templates. /// /// ## Why is this bad? -/// Mako templates allow HTML/JS rendering by default and are +/// Mako templates allow HTML and JavaScript rendering by default, and are /// inherently open to XSS attacks. Ensure variables in all templates are -/// properly sanitized via the 'n', 'h' or 'x' flags (depending on context). -/// For example, to HTML escape the variable 'data' do ${ data |h }. +/// properly sanitized via the `n`, `h` or `x` flags (depending on context). +/// For example, to HTML escape the variable `data`, use `${ data |h }`. /// /// ## Example /// ```python @@ -38,9 +38,7 @@ impl Violation for MakoTemplates { #[derive_message_formats] fn message(&self) -> String { format!( - "Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.\ - Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).\ - For example, to HTML escape the variable 'data' do ${{ data |h }}." + "Mako templates allow HTML and JavaScript rendering by default and are inherently open to XSS attacks" ) } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap index 8b504138b4cecc..fa943d7acf9380 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S702_S702.py.snap @@ -1,31 +1,28 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S702.py:6:1: S702 Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).For example, to HTML escape the variable 'data' do ${ data |h }. +S702.py:6:1: S702 Mako templates allow HTML and JavaScript rendering by default and are inherently open to XSS attacks | -4 | from mako import template -5 | 6 | Template("hello") | ^^^^^^^^ S702 7 | -8 | # XXX(fletcher): for some reason, bandit is missing the one below. keeping it +8 | mako.template.Template("hern") | -S702.py:10:1: S702 Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).For example, to HTML escape the variable 'data' do ${ data |h }. - | - 8 | # XXX(fletcher): for some reason, bandit is missing the one below. keeping it - 9 | # in for now so that if it gets fixed inadvertitently we know. -10 | mako.template.Template("hern") - | ^^^^^^^^^^^^^^^^^^^^^^ S702 -11 | template.Template("hern") - | +S702.py:8:1: S702 Mako templates allow HTML and JavaScript rendering by default and are inherently open to XSS attacks + | +6 | Template("hello") +7 | +8 | mako.template.Template("hern") + | ^^^^^^^^^^^^^^^^^^^^^^ S702 +9 | template.Template("hern") + | -S702.py:11:1: S702 Mako templates allow HTML/JS rendering by default and are inherently open to XSS attacks.Ensure variables in all templates are properly sanitized via the 'n', 'h' or 'x' flags (depending on context).For example, to HTML escape the variable 'data' do ${ data |h }. - | - 9 | # in for now so that if it gets fixed inadvertitently we know. -10 | mako.template.Template("hern") -11 | template.Template("hern") - | ^^^^^^^^^^^^^^^^^ S702 - | +S702.py:9:1: S702 Mako templates allow HTML and JavaScript rendering by default and are inherently open to XSS attacks + | +8 | mako.template.Template("hern") +9 | template.Template("hern") + | ^^^^^^^^^^^^^^^^^ S702 + |