diff --git a/README.md b/README.md index 8e656b8b6d48c2..8212531cd9fe02 100644 --- a/README.md +++ b/README.md @@ -772,6 +772,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on | S107 | HardcodedPasswordDefault | Possible hardcoded password: "..." | | | S108 | HardcodedTempFile | Probable insecure usage of temporary file or directory: "..." | | | S324 | HashlibInsecureHashFunction | Probable use of insecure hash functions in `hashlib`: "..." | | +| S501 | RequestWithNoCertValidation | Probable use of `...` call with `verify=False` disabling SSL certificate checks | | | S506 | UnsafeYAMLLoad | Probable use of unsafe `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. | | ### flake8-blind-except (BLE) diff --git a/resources/test/fixtures/flake8_bandit/S501.py b/resources/test/fixtures/flake8_bandit/S501.py new file mode 100644 index 00000000000000..25f5ef41f13f78 --- /dev/null +++ b/resources/test/fixtures/flake8_bandit/S501.py @@ -0,0 +1,40 @@ +import httpx +import requests + +requests.get('https://gmail.com', timeout=30, verify=True) +requests.get('https://gmail.com', timeout=30, verify=False) +requests.post('https://gmail.com', timeout=30, verify=True) +requests.post('https://gmail.com', timeout=30, verify=False) +requests.put('https://gmail.com', timeout=30, verify=True) +requests.put('https://gmail.com', timeout=30, verify=False) +requests.delete('https://gmail.com', timeout=30, verify=True) +requests.delete('https://gmail.com', timeout=30, verify=False) +requests.patch('https://gmail.com', timeout=30, verify=True) +requests.patch('https://gmail.com', timeout=30, verify=False) +requests.options('https://gmail.com', timeout=30, verify=True) +requests.options('https://gmail.com', timeout=30, verify=False) +requests.head('https://gmail.com', timeout=30, verify=True) +requests.head('https://gmail.com', timeout=30, verify=False) + +httpx.request('GET', 'https://gmail.com', verify=True) +httpx.request('GET', 'https://gmail.com', verify=False) +httpx.get('https://gmail.com', verify=True) +httpx.get('https://gmail.com', verify=False) +httpx.options('https://gmail.com', verify=True) +httpx.options('https://gmail.com', verify=False) +httpx.head('https://gmail.com', verify=True) +httpx.head('https://gmail.com', verify=False) +httpx.post('https://gmail.com', verify=True) +httpx.post('https://gmail.com', verify=False) +httpx.put('https://gmail.com', verify=True) +httpx.put('https://gmail.com', verify=False) +httpx.patch('https://gmail.com', verify=True) +httpx.patch('https://gmail.com', verify=False) +httpx.delete('https://gmail.com', verify=True) +httpx.delete('https://gmail.com', verify=False) +httpx.stream('https://gmail.com', verify=True) +httpx.stream('https://gmail.com', verify=False) +httpx.Client() +httpx.Client(verify=False) +httpx.AsyncClient() +httpx.AsyncClient(verify=False) diff --git a/ruff.schema.json b/ruff.schema.json index 9ade1e0a9740bd..2285403e2a6978 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -932,6 +932,7 @@ "S324", "S5", "S50", + "S501", "S506", "SIM", "SIM1", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index cdde16ce763170..5f77d19752e156 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1930,6 +1930,17 @@ where self.add_check(check); } } + if self.settings.enabled.contains(&CheckCode::S501) { + if let Some(check) = flake8_bandit::checks::request_with_no_cert_validation( + func, + args, + keywords, + &self.from_imports, + &self.import_aliases, + ) { + self.add_check(check); + } + } if self.settings.enabled.contains(&CheckCode::S506) { if let Some(check) = flake8_bandit::checks::unsafe_yaml_load( func, diff --git a/src/flake8_bandit/checks/mod.rs b/src/flake8_bandit/checks/mod.rs index f2391569d63d38..0011472887a30b 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_with_no_cert_validation::request_with_no_cert_validation; 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_with_no_cert_validation; mod unsafe_yaml_load; diff --git a/src/flake8_bandit/checks/request_with_no_cert_validation.rs b/src/flake8_bandit/checks/request_with_no_cert_validation.rs new file mode 100644 index 00000000000000..8dd9fef4f721e8 --- /dev/null +++ b/src/flake8_bandit/checks/request_with_no_cert_validation.rs @@ -0,0 +1,69 @@ +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::{Check, CheckKind}; + +const REQUESTS_HTTP_VERBS: [&str; 7] = ["get", "options", "head", "post", "put", "patch", "delete"]; +const HTTPX_METHODS: [&str; 11] = [ + "get", + "options", + "head", + "post", + "put", + "patch", + "delete", + "request", + "stream", + "Client", + "AsyncClient", +]; + +/// S501 +pub fn request_with_no_cert_validation( + func: &Expr, + args: &[Expr], + keywords: &[Keyword], + from_imports: &FxHashMap<&str, FxHashSet<&str>>, + import_aliases: &FxHashMap<&str, &str>, +) -> Option { + let call_path = dealias_call_path(collect_call_paths(func), import_aliases); + let call_args = SimpleCallArgs::new(args, keywords); + + for func_name in &REQUESTS_HTTP_VERBS { + if match_call_path(&call_path, "requests", func_name, from_imports) { + if let Some(verify_arg) = call_args.get_argument("verify", None) { + if let ExprKind::Constant { + value: Constant::Bool(false), + .. + } = &verify_arg.node + { + return Some(Check::new( + CheckKind::RequestWithNoCertValidation("requests".to_string()), + Range::from_located(verify_arg), + )); + } + } + } + } + + for func_name in &HTTPX_METHODS { + if match_call_path(&call_path, "httpx", func_name, from_imports) { + if let Some(verify_arg) = call_args.get_argument("verify", None) { + if let ExprKind::Constant { + value: Constant::Bool(false), + .. + } = &verify_arg.node + { + return Some(Check::new( + CheckKind::RequestWithNoCertValidation("httpx".to_string()), + Range::from_located(verify_arg), + )); + } + } + } + } + None +} diff --git a/src/flake8_bandit/mod.rs b/src/flake8_bandit/mod.rs index 4ad881cda83615..8049d75f2b4980 100644 --- a/src/flake8_bandit/mod.rs +++ b/src/flake8_bandit/mod.rs @@ -22,6 +22,7 @@ mod tests { #[test_case(CheckCode::S107, Path::new("S107.py"); "S107")] #[test_case(CheckCode::S108, Path::new("S108.py"); "S108")] #[test_case(CheckCode::S324, Path::new("S324.py"); "S324")] + #[test_case(CheckCode::S501, Path::new("S501.py"); "S501")] #[test_case(CheckCode::S506, Path::new("S506.py"); "S506")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); diff --git a/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S501_S501.py.snap b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S501_S501.py.snap new file mode 100644 index 00000000000000..5621a5e79943cf --- /dev/null +++ b/src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S501_S501.py.snap @@ -0,0 +1,185 @@ +--- +source: src/flake8_bandit/mod.rs +expression: checks +--- +- kind: + RequestWithNoCertValidation: requests + location: + row: 5 + column: 53 + end_location: + row: 5 + column: 58 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: requests + location: + row: 7 + column: 54 + end_location: + row: 7 + column: 59 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: requests + location: + row: 9 + column: 53 + end_location: + row: 9 + column: 58 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: requests + location: + row: 11 + column: 56 + end_location: + row: 11 + column: 61 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: requests + location: + row: 13 + column: 55 + end_location: + row: 13 + column: 60 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: requests + location: + row: 15 + column: 57 + end_location: + row: 15 + column: 62 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: requests + location: + row: 17 + column: 54 + end_location: + row: 17 + column: 59 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 20 + column: 49 + end_location: + row: 20 + column: 54 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 22 + column: 38 + end_location: + row: 22 + column: 43 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 24 + column: 42 + end_location: + row: 24 + column: 47 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 26 + column: 39 + end_location: + row: 26 + column: 44 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 28 + column: 39 + end_location: + row: 28 + column: 44 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 30 + column: 38 + end_location: + row: 30 + column: 43 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 32 + column: 40 + end_location: + row: 32 + column: 45 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 34 + column: 41 + end_location: + row: 34 + column: 46 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 36 + column: 41 + end_location: + row: 36 + column: 46 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 38 + column: 20 + end_location: + row: 38 + column: 25 + fix: ~ + parent: ~ +- kind: + RequestWithNoCertValidation: httpx + location: + row: 40 + column: 25 + end_location: + row: 40 + column: 30 + fix: ~ + parent: ~ + diff --git a/src/registry.rs b/src/registry.rs index d37766906d9187..7d40576704451a 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -337,6 +337,7 @@ pub enum CheckCode { S107, S108, S324, + S501, S506, // flake8-boolean-trap FBT001, @@ -1080,6 +1081,7 @@ pub enum CheckKind { HardcodedPasswordDefault(String), HardcodedTempFile(String), HashlibInsecureHashFunction(String), + RequestWithNoCertValidation(String), UnsafeYAMLLoad(Option), // mccabe FunctionIsTooComplex(String, usize), @@ -1546,6 +1548,7 @@ impl CheckCode { CheckCode::S107 => CheckKind::HardcodedPasswordDefault("...".to_string()), CheckCode::S108 => CheckKind::HardcodedTempFile("...".to_string()), CheckCode::S324 => CheckKind::HashlibInsecureHashFunction("...".to_string()), + CheckCode::S501 => CheckKind::RequestWithNoCertValidation("...".to_string()), CheckCode::S506 => CheckKind::UnsafeYAMLLoad(None), // mccabe CheckCode::C901 => CheckKind::FunctionIsTooComplex("...".to_string(), 10), @@ -1950,6 +1953,7 @@ impl CheckCode { CheckCode::S107 => CheckCategory::Flake8Bandit, CheckCode::S108 => CheckCategory::Flake8Bandit, CheckCode::S324 => CheckCategory::Flake8Bandit, + CheckCode::S501 => CheckCategory::Flake8Bandit, CheckCode::S506 => CheckCategory::Flake8Bandit, // flake8-simplify CheckCode::SIM101 => CheckCategory::Flake8Simplify, @@ -2331,6 +2335,7 @@ impl CheckKind { CheckKind::HardcodedPasswordDefault(..) => &CheckCode::S107, CheckKind::HardcodedTempFile(..) => &CheckCode::S108, CheckKind::HashlibInsecureHashFunction(..) => &CheckCode::S324, + CheckKind::RequestWithNoCertValidation(..) => &CheckCode::S501, CheckKind::UnsafeYAMLLoad(..) => &CheckCode::S506, // mccabe CheckKind::FunctionIsTooComplex(..) => &CheckCode::C901, @@ -3302,6 +3307,12 @@ impl CheckKind { string.escape_debug() ) } + CheckKind::RequestWithNoCertValidation(string) => { + format!( + "Probable use of `{string}` call with `verify=False` disabling SSL \ + certificate checks" + ) + } CheckKind::UnsafeYAMLLoad(loader) => match loader { Some(name) => { format!(