diff --git a/README.md b/README.md index d34e0334acd92..ffd3fcb089af9 100644 --- a/README.md +++ b/README.md @@ -762,6 +762,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on | ---- | ---- | ------- | --- | | S101 | AssertUsed | Use of `assert` detected | | | S102 | ExecUsed | Use of `exec` detected | | +| S103 | BadFilePermissions | `os.chmod` setting a permissive mask `0o777` on file or directory | | | S104 | HardcodedBindAllInterfaces | Possible binding to all interfaces | | | S105 | HardcodedPasswordString | Possible hardcoded password: `"..."` | | | S106 | HardcodedPasswordFuncArg | Possible hardcoded password: `"..."` | | diff --git a/resources/test/fixtures/flake8_bandit/S103.py b/resources/test/fixtures/flake8_bandit/S103.py new file mode 100644 index 0000000000000..8f95808bb61c2 --- /dev/null +++ b/resources/test/fixtures/flake8_bandit/S103.py @@ -0,0 +1,22 @@ +import os +import stat + +keyfile = "foo" + +os.chmod("/etc/passwd", 0o227) # Error +os.chmod("/etc/passwd", 0o7) # Error +os.chmod("/etc/passwd", 0o664) # OK +os.chmod("/etc/passwd", 0o777) # Error +os.chmod("/etc/passwd", 0o770) # Error +os.chmod("/etc/passwd", 0o776) # Error +os.chmod("/etc/passwd", 0o760) # OK +os.chmod("~/.bashrc", 511) # Error +os.chmod("/etc/hosts", 0o777) # Error +os.chmod("/tmp/oh_hai", 0x1FF) # Error +os.chmod("/etc/passwd", stat.S_IRWXU) # OK +os.chmod(keyfile, 0o777) # Error +os.chmod(keyfile, 0o7 | 0o70 | 0o700) # Error +os.chmod(keyfile, stat.S_IRWXO | stat.S_IRWXG | stat.S_IRWXU) # Error +os.chmod("~/hidden_exec", stat.S_IXGRP) # Error +os.chmod("~/hidden_exec", stat.S_IXOTH) # OK +os.chmod("/etc/passwd", stat.S_IWOTH) # Error diff --git a/ruff.schema.json b/ruff.schema.json index 94a19e6972607..4088e55eea242 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -889,6 +889,7 @@ "S10", "S101", "S102", + "S103", "S104", "S105", "S106", diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index 4714f5e075637..78a0d88edf2d8 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -574,6 +574,53 @@ pub fn followed_by_multi_statement_line(stmt: &Stmt, locator: &SourceCodeLocator match_trailing_content(stmt, locator) } +#[derive(Default)] +/// A simple representation of a call's positional and keyword arguments. +pub struct SimpleCallArgs<'a> { + pub args: Vec<&'a Expr>, + pub kwargs: FxHashMap<&'a str, &'a Expr>, +} + +impl<'a> SimpleCallArgs<'a> { + pub fn new(args: &'a Vec, keywords: &'a Vec) -> Self { + let mut result = SimpleCallArgs::default(); + + for arg in args { + match &arg.node { + ExprKind::Starred { .. } => { + break; + } + _ => { + result.args.push(arg); + } + } + } + + for keyword in keywords { + if let Some(arg) = &keyword.node.arg { + result.kwargs.insert(arg, &keyword.node.value); + } + } + + result + } + + /// Get the argument with the given name or position. + /// If the argument is not found with either name or position, return + /// `None`. + pub fn get_argument(&self, name: &'a str, position: Option) -> Option<&'a Expr> { + if let Some(kwarg) = self.kwargs.get(name) { + return Some(kwarg); + } + if let Some(position) = position { + if position < self.args.len() { + return Some(self.args[position]); + } + } + None + } +} + #[cfg(test)] mod tests { use anyhow::Result; diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 16cd4d672c75d..d0b88a07b7288 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1868,6 +1868,17 @@ where self.add_check(check); } } + if self.settings.enabled.contains(&CheckCode::S103) { + if let Some(check) = flake8_bandit::plugins::bad_file_permissions( + func, + args, + keywords, + &self.from_imports, + &self.import_aliases, + ) { + self.add_check(check); + } + } if self.settings.enabled.contains(&CheckCode::S106) { self.add_checks( flake8_bandit::plugins::hardcoded_password_func_arg(keywords).into_iter(), diff --git a/src/flake8_bandit/mod.rs b/src/flake8_bandit/mod.rs index 685e41de27797..7efdf950087eb 100644 --- a/src/flake8_bandit/mod.rs +++ b/src/flake8_bandit/mod.rs @@ -3,7 +3,6 @@ pub mod plugins; #[cfg(test)] mod tests { - use std::convert::AsRef; use std::path::Path; use anyhow::Result; @@ -15,6 +14,7 @@ mod tests { #[test_case(CheckCode::S101, Path::new("S101.py"); "S101")] #[test_case(CheckCode::S102, Path::new("S102.py"); "S102")] + #[test_case(CheckCode::S103, Path::new("S103.py"); "S103")] #[test_case(CheckCode::S104, Path::new("S104.py"); "S104")] #[test_case(CheckCode::S105, Path::new("S105.py"); "S105")] #[test_case(CheckCode::S106, Path::new("S106.py"); "S106")] diff --git a/src/flake8_bandit/plugins/bad_file_permissions.rs b/src/flake8_bandit/plugins/bad_file_permissions.rs new file mode 100644 index 0000000000000..0efb572bd29fb --- /dev/null +++ b/src/flake8_bandit/plugins/bad_file_permissions.rs @@ -0,0 +1,108 @@ +use num_traits::ToPrimitive; +use once_cell::sync::Lazy; +use rustc_hash::{FxHashMap, FxHashSet}; +use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Operator}; + +use crate::ast::helpers::{compose_call_path, match_module_member, SimpleCallArgs}; +use crate::ast::types::Range; +use crate::registry::{Check, CheckKind}; + +const WRITE_WORLD: u16 = 0o2; +const EXECUTE_GROUP: u16 = 0o10; + +static PYSTAT_MAPPING: Lazy> = Lazy::new(|| { + FxHashMap::from_iter([ + ("stat.ST_MODE", 0o0), + ("stat.S_IFDOOR", 0o0), + ("stat.S_IFPORT", 0o0), + ("stat.ST_INO", 0o1), + ("stat.S_IXOTH", 0o1), + ("stat.UF_NODUMP", 0o1), + ("stat.ST_DEV", 0o2), + ("stat.S_IWOTH", 0o2), + ("stat.UF_IMMUTABLE", 0o2), + ("stat.ST_NLINK", 0o3), + ("stat.ST_UID", 0o4), + ("stat.S_IROTH", 0o4), + ("stat.UF_APPEND", 0o4), + ("stat.ST_GID", 0o5), + ("stat.ST_SIZE", 0o6), + ("stat.ST_ATIME", 0o7), + ("stat.S_IRWXO", 0o7), + ("stat.ST_MTIME", 0o10), + ("stat.S_IXGRP", 0o10), + ("stat.UF_OPAQUE", 0o10), + ("stat.ST_CTIME", 0o11), + ("stat.S_IWGRP", 0o20), + ("stat.UF_NOUNLINK", 0o20), + ("stat.S_IRGRP", 0o40), + ("stat.UF_COMPRESSED", 0o40), + ("stat.S_IRWXG", 0o70), + ("stat.S_IEXEC", 0o100), + ("stat.S_IXUSR", 0o100), + ("stat.S_IWRITE", 0o200), + ("stat.S_IWUSR", 0o200), + ("stat.S_IREAD", 0o400), + ("stat.S_IRUSR", 0o400), + ("stat.S_IRWXU", 0o700), + ("stat.S_ISVTX", 0o1000), + ("stat.S_ISGID", 0o2000), + ("stat.S_ENFMT", 0o2000), + ("stat.S_ISUID", 0o4000), + ]) +}); + +fn get_int_value(expr: &Expr) -> Option { + match &expr.node { + ExprKind::Constant { + value: Constant::Int(value), + .. + } => value.to_u16(), + ExprKind::Attribute { .. } => { + if let Some(path) = compose_call_path(expr) { + PYSTAT_MAPPING.get(path.as_str()).copied() + } else { + None + } + } + ExprKind::BinOp { left, op, right } => { + if let (Some(left_value), Some(right_value)) = + (get_int_value(left), get_int_value(right)) + { + match op { + Operator::BitAnd => Some(left_value & right_value), + Operator::BitOr => Some(left_value | right_value), + Operator::BitXor => Some(left_value ^ right_value), + _ => None, + } + } else { + None + } + } + _ => None, + } +} + +/// S103 +pub fn bad_file_permissions( + func: &Expr, + args: &Vec, + keywords: &Vec, + from_imports: &FxHashMap<&str, FxHashSet<&str>>, + import_aliases: &FxHashMap<&str, &str>, +) -> Option { + if match_module_member(func, "os", "chmod", from_imports, import_aliases) { + let call_args = SimpleCallArgs::new(args, keywords); + if let Some(mode_arg) = call_args.get_argument("mode", Some(1)) { + if let Some(int_value) = get_int_value(mode_arg) { + if (int_value & WRITE_WORLD > 0) || (int_value & EXECUTE_GROUP > 0) { + return Some(Check::new( + CheckKind::BadFilePermissions(int_value), + Range::from_located(mode_arg), + )); + } + } + } + } + None +} diff --git a/src/flake8_bandit/plugins/mod.rs b/src/flake8_bandit/plugins/mod.rs index ab917c5a2285e..d5189a5c2a82f 100644 --- a/src/flake8_bandit/plugins/mod.rs +++ b/src/flake8_bandit/plugins/mod.rs @@ -1,4 +1,5 @@ pub use assert_used::assert_used; +pub use bad_file_permissions::bad_file_permissions; pub use exec_used::exec_used; pub use hardcoded_bind_all_interfaces::hardcoded_bind_all_interfaces; pub use hardcoded_password_default::hardcoded_password_default; @@ -8,6 +9,7 @@ pub use hardcoded_password_string::{ }; mod assert_used; +mod bad_file_permissions; mod exec_used; mod hardcoded_bind_all_interfaces; mod hardcoded_password_default; diff --git a/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S103_S103.py.snap b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S103_S103.py.snap new file mode 100644 index 0000000000000..5f9f3a85bd5b3 --- /dev/null +++ b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S103_S103.py.snap @@ -0,0 +1,135 @@ +--- +source: src/flake8_bandit/mod.rs +expression: checks +--- +- kind: + BadFilePermissions: 151 + location: + row: 6 + column: 24 + end_location: + row: 6 + column: 29 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 7 + location: + row: 7 + column: 24 + end_location: + row: 7 + column: 27 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 9 + column: 24 + end_location: + row: 9 + column: 29 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 504 + location: + row: 10 + column: 24 + end_location: + row: 10 + column: 29 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 510 + location: + row: 11 + column: 24 + end_location: + row: 11 + column: 29 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 13 + column: 22 + end_location: + row: 13 + column: 25 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 14 + column: 23 + end_location: + row: 14 + column: 28 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 15 + column: 24 + end_location: + row: 15 + column: 29 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 17 + column: 18 + end_location: + row: 17 + column: 23 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 18 + column: 18 + end_location: + row: 18 + column: 36 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 511 + location: + row: 19 + column: 18 + end_location: + row: 19 + column: 60 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 8 + location: + row: 20 + column: 26 + end_location: + row: 20 + column: 38 + fix: ~ + parent: ~ +- kind: + BadFilePermissions: 2 + location: + row: 22 + column: 24 + end_location: + row: 22 + column: 36 + fix: ~ + parent: ~ + diff --git a/src/flake8_pytest_style/plugins/fail.rs b/src/flake8_pytest_style/plugins/fail.rs index c39b6f7e0a1a6..39e4c24b77d6d 100644 --- a/src/flake8_pytest_style/plugins/fail.rs +++ b/src/flake8_pytest_style/plugins/fail.rs @@ -1,6 +1,7 @@ use rustpython_ast::{Expr, Keyword}; -use super::helpers::{is_empty_or_null_string, is_pytest_fail, SimpleCallArgs}; +use super::helpers::{is_empty_or_null_string, is_pytest_fail}; +use crate::ast::helpers::SimpleCallArgs; use crate::ast::types::Range; use crate::checkers::ast::Checker; use crate::registry::{Check, CheckKind}; diff --git a/src/flake8_pytest_style/plugins/helpers.rs b/src/flake8_pytest_style/plugins/helpers.rs index 14046c29d3ff0..84c63aa464c28 100644 --- a/src/flake8_pytest_style/plugins/helpers.rs +++ b/src/flake8_pytest_style/plugins/helpers.rs @@ -1,5 +1,4 @@ use num_traits::identities::Zero; -use rustc_hash::FxHashMap; use rustpython_ast::{Constant, Expr, ExprKind, Keyword}; use crate::ast::helpers::{collect_call_paths, compose_call_path, match_module_member}; @@ -7,50 +6,6 @@ use crate::checkers::ast::Checker; const ITERABLE_INITIALIZERS: &[&str] = &["dict", "frozenset", "list", "tuple", "set"]; -#[derive(Default)] -pub struct SimpleCallArgs<'a> { - pub args: Vec<&'a Expr>, - pub kwargs: FxHashMap<&'a str, &'a Expr>, -} - -impl<'a> SimpleCallArgs<'a> { - pub fn new(args: &'a Vec, keywords: &'a Vec) -> Self { - let mut result = SimpleCallArgs::default(); - - for arg in args { - match &arg.node { - ExprKind::Starred { .. } => { - break; - } - _ => { - result.args.push(arg); - } - } - } - - for keyword in keywords { - if let Some(arg) = &keyword.node.arg { - result.kwargs.insert(arg, &keyword.node.value); - } - } - - result - } - - pub fn get_argument(&self, name: &'a str, position: Option) -> Option<&'a Expr> { - let kwarg = self.kwargs.get(name); - if let Some(kwarg) = kwarg { - return Some(kwarg); - } - if let Some(position) = position { - if position < self.args.len() { - return Some(self.args[position]); - } - } - None - } -} - pub fn get_mark_decorators(decorators: &[Expr]) -> Vec<&Expr> { decorators .iter() diff --git a/src/flake8_pytest_style/plugins/patch.rs b/src/flake8_pytest_style/plugins/patch.rs index 4c53bf1325ed5..5ec7731109fbb 100644 --- a/src/flake8_pytest_style/plugins/patch.rs +++ b/src/flake8_pytest_style/plugins/patch.rs @@ -1,8 +1,7 @@ use rustc_hash::FxHashSet; use rustpython_ast::{Expr, ExprKind, Keyword}; -use super::helpers::SimpleCallArgs; -use crate::ast::helpers::{collect_arg_names, compose_call_path}; +use crate::ast::helpers::{collect_arg_names, compose_call_path, SimpleCallArgs}; use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; diff --git a/src/registry.rs b/src/registry.rs index 4fc78ec42b407..5bf43c91246d6 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -321,6 +321,7 @@ pub enum CheckCode { // flake8-bandit S101, S102, + S103, S104, S105, S106, @@ -1052,6 +1053,7 @@ pub enum CheckKind { // flake8-bandit AssertUsed, ExecUsed, + BadFilePermissions(u16), HardcodedBindAllInterfaces, HardcodedPasswordString(String), HardcodedPasswordFuncArg(String), @@ -1502,6 +1504,7 @@ impl CheckCode { // flake8-bandit CheckCode::S101 => CheckKind::AssertUsed, CheckCode::S102 => CheckKind::ExecUsed, + CheckCode::S103 => CheckKind::BadFilePermissions(0o777), CheckCode::S104 => CheckKind::HardcodedBindAllInterfaces, CheckCode::S105 => CheckKind::HardcodedPasswordString("...".to_string()), CheckCode::S106 => CheckKind::HardcodedPasswordFuncArg("...".to_string()), @@ -1901,6 +1904,7 @@ impl CheckCode { // flake8-bandit CheckCode::S101 => CheckCategory::Flake8Bandit, CheckCode::S102 => CheckCategory::Flake8Bandit, + CheckCode::S103 => CheckCategory::Flake8Bandit, CheckCode::S104 => CheckCategory::Flake8Bandit, CheckCode::S105 => CheckCategory::Flake8Bandit, CheckCode::S106 => CheckCategory::Flake8Bandit, @@ -2262,6 +2266,7 @@ impl CheckKind { // flake8-bandit CheckKind::AssertUsed => &CheckCode::S101, CheckKind::ExecUsed => &CheckCode::S102, + CheckKind::BadFilePermissions(..) => &CheckCode::S103, CheckKind::HardcodedBindAllInterfaces => &CheckCode::S104, CheckKind::HardcodedPasswordString(..) => &CheckCode::S105, CheckKind::HardcodedPasswordFuncArg(..) => &CheckCode::S106, @@ -3176,6 +3181,9 @@ impl CheckKind { // flake8-bandit CheckKind::AssertUsed => "Use of `assert` detected".to_string(), CheckKind::ExecUsed => "Use of `exec` detected".to_string(), + CheckKind::BadFilePermissions(mask) => { + format!("`os.chmod` setting a permissive mask `{mask:#o}` on file or directory",) + } CheckKind::HardcodedBindAllInterfaces => { "Possible binding to all interfaces".to_string() } diff --git a/src/registry_gen.rs b/src/registry_gen.rs index bc1553f714d0b..df72ca607a546 100644 --- a/src/registry_gen.rs +++ b/src/registry_gen.rs @@ -509,6 +509,7 @@ pub enum CheckCodePrefix { S10, S101, S102, + S103, S104, S105, S106, @@ -901,6 +902,7 @@ impl CheckCodePrefix { CheckCode::ERA001, CheckCode::S101, CheckCode::S102, + CheckCode::S103, CheckCode::S104, CheckCode::S105, CheckCode::S106, @@ -2587,6 +2589,7 @@ impl CheckCodePrefix { CheckCodePrefix::S => vec![ CheckCode::S101, CheckCode::S102, + CheckCode::S103, CheckCode::S104, CheckCode::S105, CheckCode::S106, @@ -2595,6 +2598,7 @@ impl CheckCodePrefix { CheckCodePrefix::S1 => vec![ CheckCode::S101, CheckCode::S102, + CheckCode::S103, CheckCode::S104, CheckCode::S105, CheckCode::S106, @@ -2603,6 +2607,7 @@ impl CheckCodePrefix { CheckCodePrefix::S10 => vec![ CheckCode::S101, CheckCode::S102, + CheckCode::S103, CheckCode::S104, CheckCode::S105, CheckCode::S106, @@ -2610,6 +2615,7 @@ impl CheckCodePrefix { ], CheckCodePrefix::S101 => vec![CheckCode::S101], CheckCodePrefix::S102 => vec![CheckCode::S102], + CheckCodePrefix::S103 => vec![CheckCode::S103], CheckCodePrefix::S104 => vec![CheckCode::S104], CheckCodePrefix::S105 => vec![CheckCode::S105], CheckCodePrefix::S106 => vec![CheckCode::S106], @@ -3583,6 +3589,7 @@ impl CheckCodePrefix { CheckCodePrefix::S10 => SuffixLength::Two, CheckCodePrefix::S101 => SuffixLength::Three, CheckCodePrefix::S102 => SuffixLength::Three, + CheckCodePrefix::S103 => SuffixLength::Three, CheckCodePrefix::S104 => SuffixLength::Three, CheckCodePrefix::S105 => SuffixLength::Three, CheckCodePrefix::S106 => SuffixLength::Three,