Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-bandit] Add Rule for S501 (request call with verify=False) #1695

Merged
merged 2 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
40 changes: 40 additions & 0 deletions resources/test/fixtures/flake8_bandit/S501.py
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,7 @@
"S324",
"S5",
"S50",
"S501",
"S506",
"SIM",
"SIM1",
Expand Down
11 changes: 11 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/flake8_bandit/checks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
69 changes: 69 additions & 0 deletions src/flake8_bandit/checks/request_with_no_cert_validation.rs
Original file line number Diff line number Diff line change
@@ -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<Check> {
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
}
1 change: 1 addition & 0 deletions src/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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: ~

Loading