From 3650aaa8b306078b76043df9ad725e703705cc4c Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Thu, 6 Jul 2023 18:46:16 +0100 Subject: [PATCH] Add documentation to the `S1XX` rules (#5479) ## Summary Add documentation to the `S1XX` rules (the `flake8-bandit` ['misc tests'](https://bandit.readthedocs.io/en/latest/plugins/index.html#plugin-id-groupings) rule group). ## Test Plan `python scripts/check_docs_formatted.py && mkdocs serve` --- .../rules/bad_file_permissions.rs | 27 ++++++++++++++- .../rules/flake8_bandit/rules/exec_used.rs | 15 ++++++++ .../rules/hardcoded_bind_all_interfaces.rs | 21 ++++++++++++ .../rules/hardcoded_password_default.rs | 33 +++++++++++++++++- .../rules/hardcoded_password_func_arg.rs | 29 +++++++++++++++- .../rules/hardcoded_password_string.rs | 26 ++++++++++++++ .../rules/hardcoded_tmp_directory.rs | 29 ++++++++++++++++ .../rules/request_without_timeout.rs | 25 ++++++++++++++ .../rules/try_except_continue.rs | 34 +++++++++++++++++++ .../flake8_bandit/rules/try_except_pass.rs | 30 ++++++++++++++++ 10 files changed, 266 insertions(+), 3 deletions(-) diff --git a/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs b/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs index 1344db788c..8cad152de7 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/bad_file_permissions.rs @@ -9,6 +9,31 @@ use ruff_python_semantic::SemanticModel; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for files with overly permissive permissions. +/// +/// ## Why is this bad? +/// Overly permissive file permissions may allow unintended access and +/// arbitrary code execution. +/// +/// ## Example +/// ```python +/// import os +/// +/// os.chmod("/etc/secrets.txt", 0o666) # rw-rw-rw- +/// ``` +/// +/// Use instead: +/// ```python +/// import os +/// +/// os.chmod("/etc/secrets.txt", 0o600) # rw------- +/// ``` +/// +/// ## References +/// - [Python documentation: `os.chmod`](https://docs.python.org/3/library/os.html#os.chmod) +/// - [Python documentation: `stat`](https://docs.python.org/3/library/stat.html) +/// - [Common Weakness Enumeration: CWE-732](https://cwe.mitre.org/data/definitions/732.html) #[violation] pub struct BadFilePermissions { mask: u16, @@ -18,7 +43,7 @@ impl Violation for BadFilePermissions { #[derive_message_formats] fn message(&self) -> String { let BadFilePermissions { mask } = self; - format!("`os.chmod` setting a permissive mask `{mask:#o}` on file or directory",) + format!("`os.chmod` setting a permissive mask `{mask:#o}` on file or directory") } } diff --git a/crates/ruff/src/rules/flake8_bandit/rules/exec_used.rs b/crates/ruff/src/rules/flake8_bandit/rules/exec_used.rs index d2dfb83fb5..af0caabc1d 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/exec_used.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/exec_used.rs @@ -5,6 +5,21 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for uses of the builtin `exec` function. +/// +/// ## Why is this bad? +/// The `exec()` function is insecure as it allows for arbitrary code +/// execution. +/// +/// ## Example +/// ```python +/// exec("print('Hello World')") +/// ``` +/// +/// ## References +/// - [Python documentation: `exec`](https://docs.python.org/3/library/functions.html#exec) +/// - [Common Weakness Enumeration: CWE-78](https://cwe.mitre.org/data/definitions/78.html) #[violation] pub struct ExecBuiltin; diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs index 86f68e10b8..ac8090395b 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs @@ -3,6 +3,27 @@ use ruff_text_size::TextRange; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +/// ## What it does +/// Checks for hardcoded bindings to all network interfaces (`0.0.0.0`). +/// +/// ## Why is this bad? +/// Binding to all network interfaces is insecure as it allows access from +/// unintended interfaces, which may be poorly secured or unauthorized. +/// +/// Instead, bind to specific interfaces. +/// +/// ## Example +/// ```python +/// ALLOWED_HOSTS = ["0.0.0.0"] +/// ``` +/// +/// Use instead: +/// ```python +/// ALLOWED_HOSTS = ["127.0.0.1", "localhost"] +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-200](https://cwe.mitre.org/data/definitions/200.html) #[violation] pub struct HardcodedBindAllInterfaces; diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_default.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_default.rs index 50be800d9e..7883d346ac 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_default.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_default.rs @@ -1,11 +1,42 @@ use rustpython_parser::ast::{Arg, ArgWithDefault, Arguments, Expr, Ranged}; -use crate::checkers::ast::Checker; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use crate::checkers::ast::Checker; + use super::super::helpers::{matches_password_name, string_literal}; +/// ## What it does +/// Checks for potential uses of hardcoded passwords in function argument +/// defaults. +/// +/// ## Why is this bad? +/// Including a hardcoded password in source code is a security risk, as an +/// attacker could discover the password and use it to gain unauthorized +/// access. +/// +/// Instead, store passwords and other secrets in configuration files, +/// environment variables, or other sources that are excluded from version +/// control. +/// +/// ## Example +/// ```python +/// def connect_to_server(password="hunter2"): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// import os +/// +/// +/// def connect_to_server(password=os.environ["PASSWORD"]): +/// ... +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-259](https://cwe.mitre.org/data/definitions/259.html) #[violation] pub struct HardcodedPasswordDefault { name: String, diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs index 1874d0e26d..5449b51171 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs @@ -1,11 +1,38 @@ use rustpython_parser::ast::{Keyword, Ranged}; -use crate::checkers::ast::Checker; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use crate::checkers::ast::Checker; + use super::super::helpers::{matches_password_name, string_literal}; +/// ## What it does +/// Checks for potential uses of hardcoded passwords in function calls. +/// +/// ## Why is this bad? +/// Including a hardcoded password in source code is a security risk, as an +/// attacker could discover the password and use it to gain unauthorized +/// access. +/// +/// Instead, store passwords and other secrets in configuration files, +/// environment variables, or other sources that are excluded from version +/// control. +/// +/// ## Example +/// ```python +/// connect_to_server(password="hunter2") +/// ``` +/// +/// Use instead: +/// ```python +/// import os +/// +/// connect_to_server(password=os.environ["PASSWORD"]) +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-259](https://cwe.mitre.org/data/definitions/259.html) #[violation] pub struct HardcodedPasswordFuncArg { name: String, diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_string.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_string.rs index e8d09125e5..c48036517f 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_string.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_string.rs @@ -7,6 +7,32 @@ use crate::checkers::ast::Checker; use super::super::helpers::{matches_password_name, string_literal}; +/// ## What it does +/// Checks for potential uses of hardcoded passwords in strings. +/// +/// ## Why is this bad? +/// Including a hardcoded password in source code is a security risk, as an +/// attacker could discover the password and use it to gain unauthorized +/// access. +/// +/// Instead, store passwords and other secrets in configuration files, +/// environment variables, or other sources that are excluded from version +/// control. +/// +/// ## Example +/// ```python +/// SECRET_KEY = "hunter2" +/// ``` +/// +/// Use instead: +/// ```python +/// import os +/// +/// SECRET_KEY = os.environ["SECRET_KEY"] +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-259](https://cwe.mitre.org/data/definitions/259.html) #[violation] pub struct HardcodedPasswordString { name: String, diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs index 8c27763f80..2fae004907 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs @@ -3,6 +3,35 @@ use rustpython_parser::ast::{Expr, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +/// ## What it does +/// Checks for the use of hardcoded temporary file or directory paths. +/// +/// ## Why is this bad? +/// The use of hardcoded paths for temporary files can be insecure. If an +/// attacker discovers the location of a hardcoded path, they can replace the +/// contents of the file or directory with a malicious payload. +/// +/// Other programs may also read or write contents to these hardcoded paths, +/// causing unexpected behavior. +/// +/// ## Example +/// ```python +/// with open("/tmp/foo.txt", "w") as file: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// import tempfile +/// +/// with tempfile.NamedTemporaryFile() as file: +/// ... +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-377](https://cwe.mitre.org/data/definitions/377.html) +/// - [Common Weakness Enumeration: CWE-379](https://cwe.mitre.org/data/definitions/379.html) +/// - [Python documentation: `tempfile`](https://docs.python.org/3/library/tempfile.html) #[violation] pub struct HardcodedTempFile { string: String, diff --git a/crates/ruff/src/rules/flake8_bandit/rules/request_without_timeout.rs b/crates/ruff/src/rules/flake8_bandit/rules/request_without_timeout.rs index 08736d5fc9..e62e92687c 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/request_without_timeout.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/request_without_timeout.rs @@ -6,6 +6,31 @@ use ruff_python_ast::helpers::{is_const_none, SimpleCallArgs}; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for uses of the Python `requests` module that omit the `timeout` +/// parameter. +/// +/// ## Why is this bad? +/// The `timeout` parameter is used to set the maximum time to wait for a +/// response from the server. By omitting the `timeout` parameter, the program +/// may hang indefinitely while awaiting a response. +/// +/// ## Example +/// ```python +/// import requests +/// +/// requests.get("https://www.example.com/") +/// ``` +/// +/// Use instead: +/// ```python +/// import requests +/// +/// requests.get("https://www.example.com/", timeout=10) +/// ``` +/// +/// ## References +/// - [Requests documentation: Timeouts](https://requests.readthedocs.io/en/latest/user/advanced/#timeouts) #[violation] pub struct RequestWithoutTimeout { implicit: bool, diff --git a/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs b/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs index 5a4e4faec4..82cc379cfb 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs @@ -6,6 +6,40 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; use crate::rules::flake8_bandit::helpers::is_untyped_exception; +/// ## What it does +/// Checks for uses of the `try`-`except`-`continue` pattern. +/// +/// ## Why is this bad? +/// The `try`-`except`-`continue` pattern suppresses all exceptions. +/// Suppressing exceptions may hide errors that could otherwise reveal +/// unexpected behavior, security vulnerabilities, or malicious activity. +/// Instead, consider logging the exception. +/// +/// ## Example +/// ```python +/// import logging +/// +/// while predicate: +/// try: +/// ... +/// except Exception: +/// continue +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// while predicate: +/// try: +/// ... +/// except Exception as exc: +/// logging.exception("Error occurred") +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-703](https://cwe.mitre.org/data/definitions/703.html) +/// - [Python documentation: `logging`](https://docs.python.org/3/library/logging.html) #[violation] pub struct TryExceptContinue; diff --git a/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs b/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs index ee4bd533d3..ba4e1d713d 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs @@ -6,6 +6,36 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; use crate::rules::flake8_bandit::helpers::is_untyped_exception; +/// ## What it does +/// Checks for uses of the `try`-`except`-`pass` pattern. +/// +/// ## Why is this bad? +/// The `try`-`except`-`pass` pattern suppresses all exceptions. Suppressing +/// exceptions may hide errors that could otherwise reveal unexpected behavior, +/// security vulnerabilities, or malicious activity. Instead, consider logging +/// the exception. +/// +/// ## Example +/// ```python +/// try: +/// ... +/// except Exception: +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// import logging +/// +/// try: +/// ... +/// except Exception as exc: +/// logging.exception("Exception occurred") +/// ``` +/// +/// ## References +/// - [Common Weakness Enumeration: CWE-703](https://cwe.mitre.org/data/definitions/703.html) +/// - [Python documentation: `logging`](https://docs.python.org/3/library/logging.html) #[violation] pub struct TryExceptPass;