Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,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)

Expand Down
29 changes: 29 additions & 0 deletions resources/test/fixtures/flake8_bandit/S701.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,9 @@
"S506",
"S508",
"S509",
"S7",
"S70",
"S701",
"SIM",
"SIM1",
"SIM10",
Expand Down
11 changes: 11 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions src/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
57 changes: 57 additions & 0 deletions src/flake8_bandit/rules/jinja2_autoescape_false.rs
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 2 additions & 0 deletions src/flake8_bandit/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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: ~

3 changes: 2 additions & 1 deletion src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 22 additions & 0 deletions src/violations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
);
Expand Down