diff --git a/README.md b/README.md index 8ae76e798902f..3a04e327b2661 100644 --- a/README.md +++ b/README.md @@ -773,6 +773,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on | 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`: "..." | | +| 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 0000000000000..25f5ef41f13f7 --- /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 9729f1d8e6e31..fa816fc9b0c57 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -934,6 +934,7 @@ "S324", "S5", "S50", + "S501", "S506", "SIM", "SIM1", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 0695f806bd394..5d5830536297c 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1938,6 +1938,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 50dad2e2b4d02..288cd1a06b85a 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 request_without_timeout::request_without_timeout; pub use unsafe_yaml_load::unsafe_yaml_load; @@ -21,5 +22,6 @@ 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 request_without_timeout; 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 0000000000000..8dd9fef4f721e --- /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 e93f41596be6c..f7ccc5bebaf61 100644 --- a/src/flake8_bandit/mod.rs +++ b/src/flake8_bandit/mod.rs @@ -23,6 +23,7 @@ mod tests { #[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::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 0000000000000..5621a5e79943c --- /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 12b0525388d91..bef2426ccfdee 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -342,6 +342,7 @@ pub enum CheckCode { S108, S113, S324, + S501, S506, // flake8-boolean-trap FBT001, @@ -1090,6 +1091,7 @@ pub enum CheckKind { HardcodedTempFile(String), HashlibInsecureHashFunction(String), RequestWithoutTimeout(Option), + RequestWithNoCertValidation(String), UnsafeYAMLLoad(Option), // mccabe FunctionIsTooComplex(String, usize), @@ -1567,6 +1569,7 @@ impl CheckCode { CheckCode::S108 => CheckKind::HardcodedTempFile("...".to_string()), CheckCode::S113 => CheckKind::RequestWithoutTimeout(None), 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), @@ -1972,6 +1975,7 @@ impl CheckCode { CheckCode::S108 => CheckCategory::Flake8Bandit, CheckCode::S113 => CheckCategory::Flake8Bandit, CheckCode::S324 => CheckCategory::Flake8Bandit, + CheckCode::S501 => CheckCategory::Flake8Bandit, CheckCode::S506 => CheckCategory::Flake8Bandit, // flake8-simplify CheckCode::SIM101 => CheckCategory::Flake8Simplify, @@ -2362,6 +2366,7 @@ impl CheckKind { CheckKind::HardcodedTempFile(..) => &CheckCode::S108, CheckKind::RequestWithoutTimeout(..) => &CheckCode::S113, CheckKind::HashlibInsecureHashFunction(..) => &CheckCode::S324, + CheckKind::RequestWithNoCertValidation(..) => &CheckCode::S501, CheckKind::UnsafeYAMLLoad(..) => &CheckCode::S506, // mccabe CheckKind::FunctionIsTooComplex(..) => &CheckCode::C901, @@ -3346,6 +3351,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!(