[flake8-bandit] Add Rule for S701 (jinja2 autoescape false) (#1815)

ref: #1646

Co-authored-by: Charlie Marsh <[email protected]>

README.md

@@ -783,6 +783,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on
 | S506 | UnsafeYAMLLoad | Probable use of unsafe `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. |  |
 | S508 | SnmpInsecureVersion | The use of SNMPv1 and SNMPv2 is insecure. Use SNMPv3 if able. |  |
 | S509 | SnmpWeakCryptography | You should not use SNMPv3 without encryption. `noAuthNoPriv` & `authNoPriv` is insecure. |  |
+| S701 | Jinja2AutoescapeFalse | By default, jinja2 sets `autoescape` to `False`. Consider using `autoescape=True` or the `select_autoescape` function to mitigate XSS vulnerabilities. |  |
 
 ### flake8-blind-except (BLE)
 

resources/test/fixtures/flake8_bandit/S701.py

@@ -0,0 +1,29 @@
+import jinja2
+from jinja2 import Environment, select_autoescape
+templateLoader = jinja2.FileSystemLoader( searchpath="/" )
+something = ''
+
+Environment(loader=templateLoader, load=templateLoader, autoescape=True)
+templateEnv = jinja2.Environment(autoescape=True,
+        loader=templateLoader )
+Environment(loader=templateLoader, load=templateLoader, autoescape=something)  # S701
+templateEnv = jinja2.Environment(autoescape=False, loader=templateLoader )  # S701
+Environment(loader=templateLoader,
+            load=templateLoader,
+            autoescape=False)  # S701
+
+Environment(loader=templateLoader,  # S701
+            load=templateLoader)
+
+Environment(loader=templateLoader, autoescape=select_autoescape())
+
+Environment(loader=templateLoader,
+            autoescape=select_autoescape(['html', 'htm', 'xml']))
+
+Environment(loader=templateLoader,
+            autoescape=jinja2.select_autoescape(['html', 'htm', 'xml']))
+
+
+def fake_func():
+    return 'foobar'
+Environment(loader=templateLoader, autoescape=fake_func())  # S701

ruff.schema.json

@@ -1515,6 +1515,9 @@
         "S506",
         "S508",
         "S509",
+        "S7",
+        "S70",
+        "S701",
         "SIM",
         "SIM1",
         "SIM10",

src/checkers/ast.rs

@@ -2023,6 +2023,17 @@ where
                         self.diagnostics.push(diagnostic);
                     }
                 }
+                if self.settings.enabled.contains(&RuleCode::S701) {
+                    if let Some(diagnostic) = flake8_bandit::rules::jinja2_autoescape_false(
+                        func,
+                        args,
+                        keywords,
+                        &self.from_imports,
+                        &self.import_aliases,
+                    ) {
+                        self.diagnostics.push(diagnostic);
+                    }
+                }
                 if self.settings.enabled.contains(&RuleCode::S106) {
                     self.diagnostics
                         .extend(flake8_bandit::rules::hardcoded_password_func_arg(keywords));

src/flake8_bandit/mod.rs

@@ -28,6 +28,7 @@ mod tests {
     #[test_case(RuleCode::S506, Path::new("S506.py"); "S506")]
     #[test_case(RuleCode::S508, Path::new("S508.py"); "S508")]
     #[test_case(RuleCode::S509, Path::new("S509.py"); "S509")]
+    #[test_case(RuleCode::S701, Path::new("S701.py"); "S701")]
     fn rules(rule_code: RuleCode, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
         let diagnostics = test_path(

src/flake8_bandit/rules/jinja2_autoescape_false.rs

@@ -0,0 +1,57 @@
+use rustc_hash::{FxHashMap, FxHashSet};
+use rustpython_ast::{Expr, ExprKind, Keyword};
+use rustpython_parser::ast::Constant;
+
+use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path, SimpleCallArgs};
+use crate::ast::types::Range;
+use crate::registry::Diagnostic;
+use crate::violations;
+
+/// S701
+pub fn jinja2_autoescape_false(
+    func: &Expr,
+    args: &[Expr],
+    keywords: &[Keyword],
+    from_imports: &FxHashMap<&str, FxHashSet<&str>>,
+    import_aliases: &FxHashMap<&str, &str>,
+) -> Option<Diagnostic> {
+    if match_call_path(
+        &dealias_call_path(collect_call_paths(func), import_aliases),
+        "jinja2",
+        "Environment",
+        from_imports,
+    ) {
+        let call_args = SimpleCallArgs::new(args, keywords);
+
+        if let Some(autoescape_arg) = call_args.get_argument("autoescape", None) {
+            match &autoescape_arg.node {
+                ExprKind::Constant {
+                    value: Constant::Bool(true),
+                    ..
+                } => (),
+                ExprKind::Call { func, .. } => {
+                    if let ExprKind::Name { id, .. } = &func.node {
+                        if id.as_str() != "select_autoescape" {
+                            return Some(Diagnostic::new(
+                                violations::Jinja2AutoescapeFalse(true),
+                                Range::from_located(autoescape_arg),
+                            ));
+                        }
+                    }
+                }
+                _ => {
+                    return Some(Diagnostic::new(
+                        violations::Jinja2AutoescapeFalse(true),
+                        Range::from_located(autoescape_arg),
+                    ))
+                }
+            }
+        } else {
+            return Some(Diagnostic::new(
+                violations::Jinja2AutoescapeFalse(false),
+                Range::from_located(func),
+            ));
+        }
+    }
+    None
+}

src/flake8_bandit/rules/mod.rs

@@ -9,6 +9,7 @@ pub use hardcoded_password_string::{
 };
 pub use hardcoded_tmp_directory::hardcoded_tmp_directory;
 pub use hashlib_insecure_hash_functions::hashlib_insecure_hash_functions;
+pub use jinja2_autoescape_false::jinja2_autoescape_false;
 pub use request_with_no_cert_validation::request_with_no_cert_validation;
 pub use request_without_timeout::request_without_timeout;
 pub use snmp_insecure_version::snmp_insecure_version;
@@ -24,6 +25,7 @@ mod hardcoded_password_func_arg;
 mod hardcoded_password_string;
 mod hardcoded_tmp_directory;
 mod hashlib_insecure_hash_functions;
+mod jinja2_autoescape_false;
 mod request_with_no_cert_validation;
 mod request_without_timeout;
 mod snmp_insecure_version;

src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S701_S701.py.snap

@@ -0,0 +1,55 @@
+---
+source: src/flake8_bandit/mod.rs
+expression: diagnostics
+---
+- kind:
+    Jinja2AutoescapeFalse: true
+  location:
+    row: 9
+    column: 67
+  end_location:
+    row: 9
+    column: 76
+  fix: ~
+  parent: ~
+- kind:
+    Jinja2AutoescapeFalse: true
+  location:
+    row: 10
+    column: 44
+  end_location:
+    row: 10
+    column: 49
+  fix: ~
+  parent: ~
+- kind:
+    Jinja2AutoescapeFalse: true
+  location:
+    row: 13
+    column: 23
+  end_location:
+    row: 13
+    column: 28
+  fix: ~
+  parent: ~
+- kind:
+    Jinja2AutoescapeFalse: false
+  location:
+    row: 15
+    column: 0
+  end_location:
+    row: 15
+    column: 11
+  fix: ~
+  parent: ~
+- kind:
+    Jinja2AutoescapeFalse: true
+  location:
+    row: 29
+    column: 46
+  end_location:
+    row: 29
+    column: 57
+  fix: ~
+  parent: ~
+

src/registry.rs

@@ -430,8 +430,9 @@ define_rule_mapping!(
     S324 => violations::HashlibInsecureHashFunction,
     S501 => violations::RequestWithNoCertValidation,
     S506 => violations::UnsafeYAMLLoad,
-        S508 => violations::SnmpInsecureVersion,
+    S508 => violations::SnmpInsecureVersion,
     S509 => violations::SnmpWeakCryptography,
+    S701 => violations::Jinja2AutoescapeFalse,
     // flake8-boolean-trap
     FBT001 => violations::BooleanPositionalArgInFunctionDefinition,
     FBT002 => violations::BooleanDefaultValueInFunctionDefinition,

src/violations.rs

@@ -4704,6 +4704,28 @@ impl AlwaysAutofixableViolation for CommentedOutCode {
 
 // flake8-bandit
 
+define_violation!(
+    pub struct Jinja2AutoescapeFalse(pub bool);
+);
+impl Violation for Jinja2AutoescapeFalse {
+    fn message(&self) -> String {
+        let Jinja2AutoescapeFalse(value) = self;
+        match value {
+            true => "Using jinja2 templates with `autoescape=False` is dangerous and can lead to \
+                     XSS. Ensure `autoescape=True` or use the `select_autoescape` function."
+                .to_string(),
+            false => "By default, jinja2 sets `autoescape` to `False`. Consider using \
+                      `autoescape=True` or the `select_autoescape` function to mitigate XSS \
+                      vulnerabilities."
+                .to_string(),
+        }
+    }
+
+    fn placeholder() -> Self {
+        Jinja2AutoescapeFalse(false)
+    }
+}
+
 define_violation!(
     pub struct AssertUsed;
 );