From c7d324c6842750c0601dbf0f0b5aafcc20fbd797 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Fri, 6 Jan 2023 20:47:49 +0600 Subject: [PATCH 1/2] [`flake8-bandit`] Add Rule for `S113` (requests call without timeout) --- README.md | 1 + resources/test/fixtures/flake8_bandit/S113.py | 23 +++ ruff.schema.json | 2 + src/checkers/ast.rs | 11 ++ src/flake8_bandit/checks/mod.rs | 2 + .../checks/request_without_timeout.rs | 45 ++++++ src/flake8_bandit/mod.rs | 1 + ...f__flake8_bandit__tests__S113_S113.py.snap | 145 ++++++++++++++++++ src/registry.rs | 11 ++ 9 files changed, 241 insertions(+) create mode 100644 resources/test/fixtures/flake8_bandit/S113.py create mode 100644 src/flake8_bandit/checks/request_without_timeout.rs create mode 100644 src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S113_S113.py.snap diff --git a/README.md b/README.md index 8e656b8b6d48c..a01e38178a867 100644 --- a/README.md +++ b/README.md @@ -771,6 +771,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on | S106 | HardcodedPasswordFuncArg | Possible hardcoded password: "..." | | | S107 | HardcodedPasswordDefault | Possible hardcoded password: "..." | | | S108 | HardcodedTempFile | Probable insecure usage of temporary file or directory: "..." | | +| S113 | RequestWithoutTimeout | Probable use of requests call without timeout. | | | S324 | HashlibInsecureHashFunction | Probable use of insecure hash functions in `hashlib`: "..." | | | S506 | UnsafeYAMLLoad | Probable use of unsafe `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. | | diff --git a/resources/test/fixtures/flake8_bandit/S113.py b/resources/test/fixtures/flake8_bandit/S113.py new file mode 100644 index 0000000000000..75cb5a7ff4f6c --- /dev/null +++ b/resources/test/fixtures/flake8_bandit/S113.py @@ -0,0 +1,23 @@ +import requests + +requests.get('https://gmail.com') +requests.get('https://gmail.com', timeout=None) +requests.get('https://gmail.com', timeout=5) +requests.post('https://gmail.com') +requests.post('https://gmail.com', timeout=None) +requests.post('https://gmail.com', timeout=5) +requests.put('https://gmail.com') +requests.put('https://gmail.com', timeout=None) +requests.put('https://gmail.com', timeout=5) +requests.delete('https://gmail.com') +requests.delete('https://gmail.com', timeout=None) +requests.delete('https://gmail.com', timeout=5) +requests.patch('https://gmail.com') +requests.patch('https://gmail.com', timeout=None) +requests.patch('https://gmail.com', timeout=5) +requests.options('https://gmail.com') +requests.options('https://gmail.com', timeout=None) +requests.options('https://gmail.com', timeout=5) +requests.head('https://gmail.com') +requests.head('https://gmail.com', timeout=None) +requests.head('https://gmail.com', timeout=5) diff --git a/ruff.schema.json b/ruff.schema.json index 9ade1e0a9740b..0f69e1781551a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -927,6 +927,8 @@ "S106", "S107", "S108", + "S11", + "S113", "S3", "S32", "S324", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index cdde16ce76317..2cbe0824c7ae6 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1957,6 +1957,17 @@ where self.add_check(check); } } + if self.settings.enabled.contains(&CheckCode::S113) { + if let Some(check) = flake8_bandit::checks::request_without_timeout( + func, + args, + keywords, + &self.from_imports, + &self.import_aliases, + ) { + self.add_check(check); + } + } // flake8-comprehensions if self.settings.enabled.contains(&CheckCode::C400) { diff --git a/src/flake8_bandit/checks/mod.rs b/src/flake8_bandit/checks/mod.rs index f2391569d63d3..50dad2e2b4d02 100644 --- a/src/flake8_bandit/checks/mod.rs +++ b/src/flake8_bandit/checks/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 request_without_timeout::request_without_timeout; pub use unsafe_yaml_load::unsafe_yaml_load; mod assert_used; @@ -20,4 +21,5 @@ mod hardcoded_password_func_arg; mod hardcoded_password_string; mod hardcoded_tmp_directory; mod hashlib_insecure_hash_functions; +mod request_without_timeout; mod unsafe_yaml_load; diff --git a/src/flake8_bandit/checks/request_without_timeout.rs b/src/flake8_bandit/checks/request_without_timeout.rs new file mode 100644 index 0000000000000..4c820ce289c7a --- /dev/null +++ b/src/flake8_bandit/checks/request_without_timeout.rs @@ -0,0 +1,45 @@ +use rustc_hash::{FxHashMap, FxHashSet}; +use rustpython_ast::{Expr, ExprKind, Keyword}; +use rustpython_parser::ast::Constant; + +use crate::ast::helpers::{match_module_member, SimpleCallArgs}; +use crate::ast::types::Range; +use crate::registry::{Check, CheckKind}; + +const HTTP_VERBS: [&str; 7] = ["get", "options", "head", "post", "put", "patch", "delete"]; + +/// S113 +pub fn request_without_timeout( + func: &Expr, + args: &[Expr], + keywords: &[Keyword], + from_imports: &FxHashMap<&str, FxHashSet<&str>>, + import_aliases: &FxHashMap<&str, &str>, +) -> Option { + for func_name in &HTTP_VERBS { + if match_module_member(func, "requests", func_name, from_imports, import_aliases) { + let call_args = SimpleCallArgs::new(args, keywords); + + if let Some(timeout_arg) = call_args.get_argument("timeout", None) { + if let Some(timeout) = match &timeout_arg.node { + ExprKind::Constant { + value: value @ Constant::None, + .. + } => Some(value.to_string()), + _ => None, + } { + return Some(Check::new( + CheckKind::RequestWithoutTimeout(Some(timeout)), + Range::from_located(timeout_arg), + )); + } + } else { + return Some(Check::new( + CheckKind::RequestWithoutTimeout(None), + Range::from_located(func), + )); + } + } + } + None +} diff --git a/src/flake8_bandit/mod.rs b/src/flake8_bandit/mod.rs index 4ad881cda8361..e93f41596be6c 100644 --- a/src/flake8_bandit/mod.rs +++ b/src/flake8_bandit/mod.rs @@ -21,6 +21,7 @@ mod tests { #[test_case(CheckCode::S106, Path::new("S106.py"); "S106")] #[test_case(CheckCode::S107, Path::new("S107.py"); "S107")] #[test_case(CheckCode::S108, Path::new("S108.py"); "S108")] + #[test_case(CheckCode::S113, Path::new("S113.py"); "S113")] #[test_case(CheckCode::S324, Path::new("S324.py"); "S324")] #[test_case(CheckCode::S506, Path::new("S506.py"); "S506")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { diff --git a/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S113_S113.py.snap b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S113_S113.py.snap new file mode 100644 index 0000000000000..76c591c2dfc0c --- /dev/null +++ b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S113_S113.py.snap @@ -0,0 +1,145 @@ +--- +source: src/flake8_bandit/mod.rs +expression: checks +--- +- kind: + RequestWithoutTimeout: ~ + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 12 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 4 + column: 42 + end_location: + row: 4 + column: 46 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: ~ + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 13 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 7 + column: 43 + end_location: + row: 7 + column: 47 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: ~ + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 12 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 10 + column: 42 + end_location: + row: 10 + column: 46 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: ~ + location: + row: 12 + column: 0 + end_location: + row: 12 + column: 15 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 13 + column: 45 + end_location: + row: 13 + column: 49 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: ~ + location: + row: 15 + column: 0 + end_location: + row: 15 + column: 14 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 16 + column: 44 + end_location: + row: 16 + column: 48 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: ~ + location: + row: 18 + column: 0 + end_location: + row: 18 + column: 16 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 19 + column: 46 + end_location: + row: 19 + column: 50 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: ~ + location: + row: 21 + column: 0 + end_location: + row: 21 + column: 13 + fix: ~ + parent: ~ +- kind: + RequestWithoutTimeout: None + location: + row: 22 + column: 43 + end_location: + row: 22 + column: 47 + fix: ~ + parent: ~ + diff --git a/src/registry.rs b/src/registry.rs index d37766906d918..d3f425478a8b3 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -336,6 +336,7 @@ pub enum CheckCode { S106, S107, S108, + S113, S324, S506, // flake8-boolean-trap @@ -1080,6 +1081,7 @@ pub enum CheckKind { HardcodedPasswordDefault(String), HardcodedTempFile(String), HashlibInsecureHashFunction(String), + RequestWithoutTimeout(Option), UnsafeYAMLLoad(Option), // mccabe FunctionIsTooComplex(String, usize), @@ -1545,6 +1547,7 @@ impl CheckCode { CheckCode::S106 => CheckKind::HardcodedPasswordFuncArg("...".to_string()), CheckCode::S107 => CheckKind::HardcodedPasswordDefault("...".to_string()), CheckCode::S108 => CheckKind::HardcodedTempFile("...".to_string()), + CheckCode::S113 => CheckKind::RequestWithoutTimeout(None), CheckCode::S324 => CheckKind::HashlibInsecureHashFunction("...".to_string()), CheckCode::S506 => CheckKind::UnsafeYAMLLoad(None), // mccabe @@ -1949,6 +1952,7 @@ impl CheckCode { CheckCode::S106 => CheckCategory::Flake8Bandit, CheckCode::S107 => CheckCategory::Flake8Bandit, CheckCode::S108 => CheckCategory::Flake8Bandit, + CheckCode::S113 => CheckCategory::Flake8Bandit, CheckCode::S324 => CheckCategory::Flake8Bandit, CheckCode::S506 => CheckCategory::Flake8Bandit, // flake8-simplify @@ -2330,6 +2334,7 @@ impl CheckKind { CheckKind::HardcodedPasswordFuncArg(..) => &CheckCode::S106, CheckKind::HardcodedPasswordDefault(..) => &CheckCode::S107, CheckKind::HardcodedTempFile(..) => &CheckCode::S108, + CheckKind::RequestWithoutTimeout(..) => &CheckCode::S113, CheckKind::HashlibInsecureHashFunction(..) => &CheckCode::S324, CheckKind::UnsafeYAMLLoad(..) => &CheckCode::S506, // mccabe @@ -3313,6 +3318,12 @@ impl CheckKind { objects. Consider `yaml.safe_load`." .to_string(), }, + CheckKind::RequestWithoutTimeout(timeout) => match timeout { + Some(value) => { + format!("Probable use of requests call with timeout set to `{value}`") + } + None => "Probable use of requests call without timeout.".to_string(), + }, // flake8-blind-except CheckKind::BlindExcept(name) => format!("Do not catch blind exception: `{name}`"), // mccabe From c6d97a49e7598fcd7d092d9d89da8ffad12572d0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 6 Jan 2023 10:25:57 -0500 Subject: [PATCH 2/2] Merge, tweak --- README.md | 2 +- src/flake8_bandit/checks/request_without_timeout.rs | 6 +++--- src/registry.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a01e38178a867..c35850d4998cb 100644 --- a/README.md +++ b/README.md @@ -771,7 +771,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on | S106 | HardcodedPasswordFuncArg | Possible hardcoded password: "..." | | | S107 | HardcodedPasswordDefault | Possible hardcoded password: "..." | | | S108 | HardcodedTempFile | Probable insecure usage of temporary file or directory: "..." | | -| S113 | RequestWithoutTimeout | Probable use of requests call without timeout. | | +| S113 | RequestWithoutTimeout | Probable use of requests call without timeout | | | S324 | HashlibInsecureHashFunction | Probable use of insecure hash functions in `hashlib`: "..." | | | S506 | UnsafeYAMLLoad | Probable use of unsafe `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. | | diff --git a/src/flake8_bandit/checks/request_without_timeout.rs b/src/flake8_bandit/checks/request_without_timeout.rs index 4c820ce289c7a..18d258d7efddc 100644 --- a/src/flake8_bandit/checks/request_without_timeout.rs +++ b/src/flake8_bandit/checks/request_without_timeout.rs @@ -2,7 +2,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use rustpython_ast::{Expr, ExprKind, Keyword}; use rustpython_parser::ast::Constant; -use crate::ast::helpers::{match_module_member, SimpleCallArgs}; +use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path, SimpleCallArgs}; use crate::ast::types::Range; use crate::registry::{Check, CheckKind}; @@ -16,10 +16,10 @@ pub fn request_without_timeout( from_imports: &FxHashMap<&str, FxHashSet<&str>>, import_aliases: &FxHashMap<&str, &str>, ) -> Option { + let call_path = dealias_call_path(collect_call_paths(func), import_aliases); for func_name in &HTTP_VERBS { - if match_module_member(func, "requests", func_name, from_imports, import_aliases) { + if match_call_path(&call_path, "requests", func_name, from_imports) { let call_args = SimpleCallArgs::new(args, keywords); - if let Some(timeout_arg) = call_args.get_argument("timeout", None) { if let Some(timeout) = match &timeout_arg.node { ExprKind::Constant { diff --git a/src/registry.rs b/src/registry.rs index d3f425478a8b3..0d92c4c1244e5 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -3322,7 +3322,7 @@ impl CheckKind { Some(value) => { format!("Probable use of requests call with timeout set to `{value}`") } - None => "Probable use of requests call without timeout.".to_string(), + None => "Probable use of requests call without timeout".to_string(), }, // flake8-blind-except CheckKind::BlindExcept(name) => format!("Do not catch blind exception: `{name}`"),