From 3526d0d90bbeb539306ad1fe4e6bb2d20a1e32d2 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Wed, 3 Jan 2024 20:59:44 +0100 Subject: [PATCH 1/4] [`flake8-bandit`] Add `S504` `SslWithNoVersion` rule --- .../test/fixtures/flake8_bandit/S504.py | 16 ++++++ .../src/checkers/ast/analyze/expression.rs | 3 ++ crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_bandit/mod.rs | 1 + .../src/rules/flake8_bandit/rules/mod.rs | 2 + .../rules/ssl_with_no_version.rs | 53 +++++++++++++++++++ ...s__flake8_bandit__tests__S504_S504.py.snap | 20 +++++++ ruff.schema.json | 1 + 8 files changed, 97 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py new file mode 100644 index 0000000000000..3113a8597338b --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py @@ -0,0 +1,16 @@ +import ssl +from ssl import wrap_socket + + +ssl.wrap_socket() # S504 +wrap_socket() # S504 +ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK + + +class Foo: + def wrap_socket(self): + pass + + +f = Foo() +f.wrap_socket() # OK \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index b0f9ce3bc70b1..8cc25e8128b9e 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -965,6 +965,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryDunderCall) { pylint::rules::unnecessary_dunder_call(checker, call); } + if checker.enabled(Rule::SslWithNoVersion) { + flake8_bandit::rules::ssl_with_no_version(checker, call); + } } Expr::Dict(dict) => { if checker.any_enabled(&[ diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 905e249e2af10..8b735b18e70ca 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -642,6 +642,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Bandit, "413") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPycryptoImport), (Flake8Bandit, "415") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPyghmiImport), (Flake8Bandit, "501") => (RuleGroup::Stable, rules::flake8_bandit::rules::RequestWithNoCertValidation), + (Flake8Bandit, "504") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslWithNoVersion), (Flake8Bandit, "505") => (RuleGroup::Preview, rules::flake8_bandit::rules::WeakCryptographicKey), (Flake8Bandit, "506") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnsafeYAMLLoad), (Flake8Bandit, "507") => (RuleGroup::Preview, rules::flake8_bandit::rules::SSHNoHostKeyVerification), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index a27f8b19444d8..5976d02af5f87 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs @@ -36,6 +36,7 @@ mod tests { #[test_case(Rule::SSHNoHostKeyVerification, Path::new("S507.py"))] #[test_case(Rule::SnmpInsecureVersion, Path::new("S508.py"))] #[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"))] + #[test_case(Rule::SslWithNoVersion, Path::new("S504.py"))] #[test_case(Rule::StartProcessWithAShell, Path::new("S605.py"))] #[test_case(Rule::StartProcessWithNoShell, Path::new("S606.py"))] #[test_case(Rule::StartProcessWithPartialPath, Path::new("S607.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs index e33ee02f098c9..672e5f6e48f45 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs @@ -20,6 +20,7 @@ pub(crate) use shell_injection::*; pub(crate) use snmp_insecure_version::*; pub(crate) use snmp_weak_cryptography::*; pub(crate) use ssh_no_host_key_verification::*; +pub(crate) use ssl_with_no_version::*; pub(crate) use suspicious_function_call::*; pub(crate) use suspicious_imports::*; pub(crate) use tarfile_unsafe_members::*; @@ -50,6 +51,7 @@ mod shell_injection; mod snmp_insecure_version; mod snmp_weak_cryptography; mod ssh_no_host_key_verification; +mod ssl_with_no_version; mod suspicious_function_call; mod suspicious_imports; mod tarfile_unsafe_members; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs new file mode 100644 index 0000000000000..60e5f2970bc5a --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs @@ -0,0 +1,53 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::ExprCall; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for calls to `ssl.wrap_socket()` with no version set +/// +/// ## Why is this bad? +/// Theis method is known to provide a default value that maximizes compatibility, but permits use +/// of the aforementioned broken protocol versions. +/// +/// ## Example +/// ```python +/// import ssl +/// +/// ssl.wrap_socket() +/// ``` +/// +/// Use instead: +/// ```python +/// import ssl +/// +/// ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) +/// ``` +#[violation] +pub struct SslWithNoVersion; + +impl Violation for SslWithNoVersion { + #[derive_message_formats] + fn message(&self) -> String { + format!("`ssl.wrap_socket` called with no `ssl_version` set`") + } +} + +/// S504 +pub(crate) fn ssl_with_no_version(checker: &mut Checker, call: &ExprCall) { + if !checker + .semantic() + .resolve_call_path(call.func.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["ssl", "wrap_socket"])) + { + return; + }; + + if call.arguments.find_keyword("ssl_version").is_none() { + checker + .diagnostics + .push(Diagnostic::new(SslWithNoVersion, call.range())); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap new file mode 100644 index 0000000000000..76b357ef2b72a --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs +--- +S504.py:5:1: S504 `ssl.wrap_socket` called with no `ssl_version` set` + | +5 | ssl.wrap_socket() # S504 + | ^^^^^^^^^^^^^^^^^ S504 +6 | wrap_socket() # S504 +7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK + | + +S504.py:6:1: S504 `ssl.wrap_socket` called with no `ssl_version` set` + | +5 | ssl.wrap_socket() # S504 +6 | wrap_socket() # S504 + | ^^^^^^^^^^^^^ S504 +7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 40d0ecb8ee5dc..c747417ed2656 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3512,6 +3512,7 @@ "S5", "S50", "S501", + "S504", "S505", "S506", "S507", From 9192877126cdcc555d51a0cf7c320125c83649da Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Wed, 3 Jan 2024 21:16:59 +0100 Subject: [PATCH 2/4] Add \n to fixture --- .../ruff_linter/resources/test/fixtures/flake8_bandit/S504.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py index 3113a8597338b..e1f1b4b893082 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py @@ -13,4 +13,4 @@ def wrap_socket(self): f = Foo() -f.wrap_socket() # OK \ No newline at end of file +f.wrap_socket() # OK From 4fc69267dd5cff014697ffb63aded13d79a31fc0 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Wed, 3 Jan 2024 21:18:52 +0100 Subject: [PATCH 3/4] Fix typos --- .../src/rules/flake8_bandit/rules/ssl_with_no_version.rs | 2 +- .../src/rules/flake8_bandit/rules/suspicious_imports.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs index 60e5f2970bc5a..1aa3f22c889b2 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs @@ -9,7 +9,7 @@ use crate::checkers::ast::Checker; /// Checks for calls to `ssl.wrap_socket()` with no version set /// /// ## Why is this bad? -/// Theis method is known to provide a default value that maximizes compatibility, but permits use +/// This method is known to provide a default value that maximizes compatibility, but permits use /// of the aforementioned broken protocol versions. /// /// ## Example diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs index e8d8bbed5a7ef..3c4269978ebb2 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs @@ -13,7 +13,7 @@ use crate::registry::AsRule; /// Checks for imports of the`telnetlib` module. /// /// ## Why is this bad? -/// Telnet is considered insecure. Instead, ise SSH or another encrypted +/// Telnet is considered insecure. Instead, use SSH or another encrypted /// protocol. /// /// ## Example From 51fb74873e43cdd83e6aabe32dac1291d3c20388 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 3 Jan 2024 16:33:51 -0500 Subject: [PATCH 4/4] Tweaks --- .../test/fixtures/flake8_bandit/S504.py | 7 +++--- crates/ruff_linter/src/registry/rule_set.rs | 2 +- .../rules/ssl_with_no_version.rs | 22 +++++++++---------- ...s__flake8_bandit__tests__S504_S504.py.snap | 18 ++++++++------- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py index e1f1b4b893082..1de96762a5a9f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py @@ -1,16 +1,15 @@ import ssl from ssl import wrap_socket - ssl.wrap_socket() # S504 wrap_socket() # S504 ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK -class Foo: +class Class: def wrap_socket(self): pass -f = Foo() -f.wrap_socket() # OK +obj = Class() +obj.wrap_socket() # OK diff --git a/crates/ruff_linter/src/registry/rule_set.rs b/crates/ruff_linter/src/registry/rule_set.rs index e5214d5e447b3..ff6561740d774 100644 --- a/crates/ruff_linter/src/registry/rule_set.rs +++ b/crates/ruff_linter/src/registry/rule_set.rs @@ -3,7 +3,7 @@ use ruff_macros::CacheKey; use std::fmt::{Debug, Formatter}; use std::iter::FusedIterator; -const RULESET_SIZE: usize = 12; +const RULESET_SIZE: usize = 13; /// A set of [`Rule`]s. /// diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs index 1aa3f22c889b2..ca8623dca8b42 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_with_no_version.rs @@ -6,11 +6,11 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for calls to `ssl.wrap_socket()` with no version set +/// Checks for calls to `ssl.wrap_socket()` without an `ssl_version`. /// /// ## Why is this bad? -/// This method is known to provide a default value that maximizes compatibility, but permits use -/// of the aforementioned broken protocol versions. +/// This method is known to provide a default value that maximizes +/// compatibility, but permits use of insecure protocols. /// /// ## Example /// ```python @@ -31,23 +31,21 @@ pub struct SslWithNoVersion; impl Violation for SslWithNoVersion { #[derive_message_formats] fn message(&self) -> String { - format!("`ssl.wrap_socket` called with no `ssl_version` set`") + format!("`ssl.wrap_socket` called without an `ssl_version``") } } /// S504 pub(crate) fn ssl_with_no_version(checker: &mut Checker, call: &ExprCall) { - if !checker + if checker .semantic() .resolve_call_path(call.func.as_ref()) .is_some_and(|call_path| matches!(call_path.as_slice(), ["ssl", "wrap_socket"])) { - return; - }; - - if call.arguments.find_keyword("ssl_version").is_none() { - checker - .diagnostics - .push(Diagnostic::new(SslWithNoVersion, call.range())); + if call.arguments.find_keyword("ssl_version").is_none() { + checker + .diagnostics + .push(Diagnostic::new(SslWithNoVersion, call.range())); + } } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap index 76b357ef2b72a..aeaa2578d2952 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S504_S504.py.snap @@ -1,20 +1,22 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S504.py:5:1: S504 `ssl.wrap_socket` called with no `ssl_version` set` +S504.py:4:1: S504 `ssl.wrap_socket` called without an `ssl_version`` | -5 | ssl.wrap_socket() # S504 +2 | from ssl import wrap_socket +3 | +4 | ssl.wrap_socket() # S504 | ^^^^^^^^^^^^^^^^^ S504 -6 | wrap_socket() # S504 -7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK +5 | wrap_socket() # S504 +6 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK | -S504.py:6:1: S504 `ssl.wrap_socket` called with no `ssl_version` set` +S504.py:5:1: S504 `ssl.wrap_socket` called without an `ssl_version`` | -5 | ssl.wrap_socket() # S504 -6 | wrap_socket() # S504 +4 | ssl.wrap_socket() # S504 +5 | wrap_socket() # S504 | ^^^^^^^^^^^^^ S504 -7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK +6 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK |