Implement flake8-bandit rule S103 (#1636)

This commit is contained in:
Edgar R. M 2023-01-04 14:47:38 -06:00 committed by GitHub
parent 8b8e6e44ea
commit 1817f8752b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 346 additions and 49 deletions

View file

@ -763,6 +763,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: `"..."` | |

View file

@ -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

View file

@ -889,6 +889,7 @@
"S10",
"S101",
"S102",
"S103",
"S104",
"S105",
"S106",

View file

@ -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<Expr>, keywords: &'a Vec<Keyword>) -> 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<usize>) -> 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;

View file

@ -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(),

View file

@ -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")]

View file

@ -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<FxHashMap<&'static str, u16>> = 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<u16> {
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<Expr>,
keywords: &Vec<Keyword>,
from_imports: &FxHashMap<&str, FxHashSet<&str>>,
import_aliases: &FxHashMap<&str, &str>,
) -> Option<Check> {
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
}

View file

@ -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;

View file

@ -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: ~

View file

@ -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};

View file

@ -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<Expr>, keywords: &'a Vec<Keyword>) -> 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<usize>) -> 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()

View file

@ -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;

View file

@ -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()
}

View file

@ -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,