[flake8-bandit] Add Rule for S113 (requests call without timeout) (…

…#1692) ref: #1646 Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
astral-sh · Jan 6, 2023 · 6aba43a · 6aba43a
1 parent 8a3a6a9
commit 6aba43a
Show file tree

Hide file tree

Showing 9 changed files with 241 additions and 0 deletions.
diff --git 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
@@ -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
@@ -927,6 +927,8 @@
         "S106",
         "S107",
         "S108",
+        "S11",
+        "S113",
         "S3",
         "S32",
         "S324",

diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs
@@ -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) {

diff --git 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
@@ -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
+}
diff --git 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
@@ -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
@@ -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<String>),
     UnsafeYAMLLoad(Option<String>),
     // 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