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 S113 (requests call without timeout) #1692

Merged
merged 3 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 @@ -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`. | |

Expand Down
23 changes: 23 additions & 0 deletions resources/test/fixtures/flake8_bandit/S113.py
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 2 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,8 @@
"S106",
"S107",
"S108",
"S11",
"S113",
"S3",
"S32",
"S324",
Expand Down
11 changes: 11 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,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) {
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_without_timeout::request_without_timeout;
pub use unsafe_yaml_load::unsafe_yaml_load;

mod assert_used;
Expand All @@ -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;
45 changes: 45 additions & 0 deletions src/flake8_bandit/checks/request_without_timeout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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 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<Check> {
let call_path = dealias_call_path(collect_call_paths(func), import_aliases);
for func_name in &HTTP_VERBS {
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 {
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
}
1 change: 1 addition & 0 deletions src/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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: ~

11 changes: 11 additions & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ pub enum CheckCode {
S106,
S107,
S108,
S113,
S324,
S506,
// flake8-boolean-trap
Expand Down Expand Up @@ -1080,6 +1081,7 @@ pub enum CheckKind {
HardcodedPasswordDefault(String),
HardcodedTempFile(String),
HashlibInsecureHashFunction(String),
RequestWithoutTimeout(Option<String>),
UnsafeYAMLLoad(Option<String>),
// mccabe
FunctionIsTooComplex(String, usize),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down