From 1f706ca990f5934055f4fce4c9e633c71a0ed973 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 4 Jan 2024 01:10:43 +0100 Subject: [PATCH 1/5] Add boilerplate, docs and fixture --- .../test/fixtures/flake8_bandit/S502.py | 26 ++++++++++ .../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_insecure_version.rs | 47 +++++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py new file mode 100644 index 0000000000000..b62b6c200d02d --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py @@ -0,0 +1,26 @@ +import ssl +from OpenSSL import SSL + +wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 +ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 +ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 +SSL.Context(method=SSL.SSLv2_METHOD) # S502 +SSL.Context(method=SSL.SSLv23_METHOD) # S502 +SSL.Context(method=SSL.SSLv3_METHOD) # S502 +SSL.Context(method=SSL.TLSv1_METHOD) # S502 + +func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 +func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 +func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 +func(method=SSL.SSLv2_METHOD) # S502 +func(method=SSL.SSLv23_METHOD) # S502 +func(method=SSL.SSLv3_METHOD) # S502 +func(method=SSL.TLSv1_METHOD) # S502 + +wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK +SSL.Context(method=SSL.TLS_SERVER_METHOD) # OK +func(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK + + + + diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 8cc25e8128b9e..4e9e75c45424d 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -968,6 +968,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::SslWithNoVersion) { flake8_bandit::rules::ssl_with_no_version(checker, call); } + if checker.enabled(Rule::SslInsecureVersion) { + flake8_bandit::rules::ssl_insecure_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 8b735b18e70ca..56e51ed65fbbf 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, "502") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslInsecureVersion), (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), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index 5976d02af5f87..e6351aa13a7da 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::SslInsecureVersoin, Path::new("S502.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"))] 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 672e5f6e48f45..cd622042f33ee 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_insecure_version::*; pub(crate) use ssl_with_no_version::*; pub(crate) use suspicious_function_call::*; pub(crate) use suspicious_imports::*; @@ -51,6 +52,7 @@ mod shell_injection; mod snmp_insecure_version; mod snmp_weak_cryptography; mod ssh_no_host_key_verification; +mod ssl_insecure_version; mod ssl_with_no_version; mod suspicious_function_call; mod suspicious_imports; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs new file mode 100644 index 0000000000000..1d2da606823bf --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs @@ -0,0 +1,47 @@ +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 Python methods with parameters that indicate the used broken SSL/TLS +/// protocol versions. +/// +/// ## Why is this bad? +/// Several highly publicized exploitable flaws have been discovered in all versions of SSL and +/// early versions of TLS. It is strongly recommended that use of the following known broken +/// protocol versions be avoided: +/// - SSL v2 +/// - SSL v3 +/// - TLS v1 +/// - TLS v1.1 +/// +/// ## Example +/// ```python +/// import ssl +/// +/// ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) +/// ``` +/// +/// Use instead: +/// ```python +/// import ssl +/// +/// ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) +/// ``` +#[violation] +pub struct SslInsecureVersion { + protocol: String +} + +impl Violation for SslInsecureVersion { + #[derive_message_formats] + fn message(&self) -> String { + format!("Call made with insecure SSL protocol: {}", self.protocol) + } +} + +/// S502 +pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) {} From 32a0faa32e3700c6dec2b70740eef2e7f88b7419 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 4 Jan 2024 12:00:39 +0100 Subject: [PATCH 2/5] [WIP] First implementation TODO: Fix borrow issues --- .../src/rules/flake8_bandit/mod.rs | 2 +- .../rules/ssl_insecure_version.rs | 88 ++++++++++++++++++- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs index e6351aa13a7da..0f79d2dbddb1b 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/mod.rs @@ -36,7 +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::SslInsecureVersoin, Path::new("S502.py"))] + #[test_case(Rule::SslInsecureVersion, Path::new("S502.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"))] diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs index 1d2da606823bf..670bf82b0ffff 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs @@ -1,7 +1,7 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::ExprCall; -use ruff_text_size::Ranged; +use ruff_python_ast::{self as ast, Expr, ExprCall}; +use ruff_python_semantic::analyze::typing::find_assigned_value; use crate::checkers::ast::Checker; @@ -33,7 +33,7 @@ use crate::checkers::ast::Checker; /// ``` #[violation] pub struct SslInsecureVersion { - protocol: String + protocol: String, } impl Violation for SslInsecureVersion { @@ -43,5 +43,85 @@ impl Violation for SslInsecureVersion { } } +const INSECURSE_SSL_PROTOCOLS: &[&str] = &[ + "PROTOCOL_SSLv2", + "PROTOCOL_SSLv3", + "PROTOCOL_TLSv1", + "PROTOCOL_TLSv1_1", + "SSLv2_METHOD", + "SSLv23_METHOD", + "SSLv3_METHOD", + "TLSv1_METHOD", + "TLSv1_1_METHOD", +]; + /// S502 -pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) {} +pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) { + let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) else { + return; + }; + + let keywords = match call_path.as_slice() { + &["ssl", "wrap_socket"] => vec!["ssl_version"], + &["OpenSSL", "SSL", "Context"] => vec!["method"], + _ => vec!["ssl_version", "method"], + }; + + let mut violations: Vec = vec![]; + + for arg in keywords { + let Some(keyword) = call.arguments.find_keyword(arg) else { + return; + }; + match &keyword.value { + Expr::Name(ast::ExprName { id, .. }) => { + let Some(val) = find_assigned_value(id, checker.semantic()) else { + continue; + }; + println!("ASSIGNED VALUE: {:?}", val); + match val { + Expr::Name(ast::ExprName { id, .. }) => { + if INSECURSE_SSL_PROTOCOLS.contains(&id.as_str()) { + violations.push(Diagnostic::new( + SslInsecureVersion { + protocol: id.to_string(), + }, + keyword.range, + )); + } + } + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if INSECURSE_SSL_PROTOCOLS.contains(&attr.as_str()) { + violations.push(Diagnostic::new( + SslInsecureVersion { + protocol: attr.to_string(), + }, + keyword.range, + )) + } + } + _ => { + continue; + } + } + } + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if INSECURSE_SSL_PROTOCOLS.contains(&attr.as_str()) { + violations.push(Diagnostic::new( + SslInsecureVersion { + protocol: attr.to_string(), + }, + keyword.range, + )) + } + } + _ => { + return; + } + } + } + + for violation in violations { + checker.diagnostics.push(violation) + } +} From b920af2b01ae3a91dd4aa4132064abcaba7b848e Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 4 Jan 2024 12:49:01 +0100 Subject: [PATCH 3/5] Update schema, fix implementation --- .../test/fixtures/flake8_bandit/S502.py | 11 +- .../rules/ssl_insecure_version.rs | 46 +++--- ...s__flake8_bandit__tests__S502_S502.py.snap | 136 ++++++++++++++++++ ruff.schema.json | 1 + 4 files changed, 166 insertions(+), 28 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py index b62b6c200d02d..1d0751b1f35e8 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py @@ -1,13 +1,20 @@ import ssl +from ssl import wrap_socket from OpenSSL import SSL +from OpenSSL.SSL import Context wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 SSL.Context(method=SSL.SSLv2_METHOD) # S502 SSL.Context(method=SSL.SSLv23_METHOD) # S502 -SSL.Context(method=SSL.SSLv3_METHOD) # S502 -SSL.Context(method=SSL.TLSv1_METHOD) # S502 +Context(method=SSL.SSLv3_METHOD) # S502 +Context(method=SSL.TLSv1_METHOD) # S502 + + +def func(): + pass + func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs index 670bf82b0ffff..09740c073a70c 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs @@ -43,7 +43,7 @@ impl Violation for SslInsecureVersion { } } -const INSECURSE_SSL_PROTOCOLS: &[&str] = &[ +const INSECURE_SSL_PROTOCOLS: &[&str] = &[ "PROTOCOL_SSLv2", "PROTOCOL_SSLv3", "PROTOCOL_TLSv1", @@ -57,32 +57,30 @@ const INSECURSE_SSL_PROTOCOLS: &[&str] = &[ /// S502 pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) { - let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) else { - return; - }; - - let keywords = match call_path.as_slice() { - &["ssl", "wrap_socket"] => vec!["ssl_version"], - &["OpenSSL", "SSL", "Context"] => vec!["method"], - _ => vec!["ssl_version", "method"], + let keywords = match checker.semantic().resolve_call_path(call.func.as_ref()) { + Some(call_path) => { + match *call_path.as_slice() { + ["ssl", "wrap_socket"] => vec!["ssl_version"], + ["OpenSSL", "SSL", "Context"] => vec!["method"], + _ => vec!["ssl_version", "method"], + } + }, + None => vec!["ssl_version", "method"] }; - let mut violations: Vec = vec![]; - for arg in keywords { let Some(keyword) = call.arguments.find_keyword(arg) else { - return; + continue; }; match &keyword.value { Expr::Name(ast::ExprName { id, .. }) => { let Some(val) = find_assigned_value(id, checker.semantic()) else { continue; }; - println!("ASSIGNED VALUE: {:?}", val); match val { Expr::Name(ast::ExprName { id, .. }) => { - if INSECURSE_SSL_PROTOCOLS.contains(&id.as_str()) { - violations.push(Diagnostic::new( + if INSECURE_SSL_PROTOCOLS.contains(&id.as_str()) { + checker.diagnostics.push(Diagnostic::new( SslInsecureVersion { protocol: id.to_string(), }, @@ -91,13 +89,13 @@ pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) { } } Expr::Attribute(ast::ExprAttribute { attr, .. }) => { - if INSECURSE_SSL_PROTOCOLS.contains(&attr.as_str()) { - violations.push(Diagnostic::new( + if INSECURE_SSL_PROTOCOLS.contains(&attr.as_str()) { + checker.diagnostics.push(Diagnostic::new( SslInsecureVersion { protocol: attr.to_string(), }, keyword.range, - )) + )); } } _ => { @@ -106,22 +104,18 @@ pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) { } } Expr::Attribute(ast::ExprAttribute { attr, .. }) => { - if INSECURSE_SSL_PROTOCOLS.contains(&attr.as_str()) { - violations.push(Diagnostic::new( + if INSECURE_SSL_PROTOCOLS.contains(&attr.as_str()) { + checker.diagnostics.push(Diagnostic::new( SslInsecureVersion { protocol: attr.to_string(), }, keyword.range, - )) + )); } } _ => { - return; + continue; } } } - - for violation in violations { - checker.diagnostics.push(violation) - } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap new file mode 100644 index 0000000000000..d3aeb02c12d10 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap @@ -0,0 +1,136 @@ +--- +source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs +--- +S502.py:6:13: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv3 + | +4 | from OpenSSL.SSL import Context +5 | +6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 +7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 +8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 + | + +S502.py:7:17: S502 Call made with insecure SSL protocol: PROTOCOL_TLSv1 + | +6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 +7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 +8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 +9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 + | + +S502.py:8:17: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv2 + | + 6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 + 7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 + 8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 + 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 +10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 + | + +S502.py:9:13: S502 Call made with insecure SSL protocol: SSLv2_METHOD + | + 7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 + 8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 + 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^ S502 +10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 +11 | Context(method=SSL.SSLv3_METHOD) # S502 + | + +S502.py:10:13: S502 Call made with insecure SSL protocol: SSLv23_METHOD + | + 8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 + 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 +10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^ S502 +11 | Context(method=SSL.SSLv3_METHOD) # S502 +12 | Context(method=SSL.TLSv1_METHOD) # S502 + | + +S502.py:11:9: S502 Call made with insecure SSL protocol: SSLv3_METHOD + | + 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 +10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 +11 | Context(method=SSL.SSLv3_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^ S502 +12 | Context(method=SSL.TLSv1_METHOD) # S502 + | + +S502.py:12:9: S502 Call made with insecure SSL protocol: TLSv1_METHOD + | +10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 +11 | Context(method=SSL.SSLv3_METHOD) # S502 +12 | Context(method=SSL.TLSv1_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^ S502 + | + +S502.py:19:6: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv2 + | +19 | func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 +20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 +21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 + | + +S502.py:20:6: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv3 + | +19 | func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 +20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 +21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 +22 | func(method=SSL.SSLv2_METHOD) # S502 + | + +S502.py:21:6: S502 Call made with insecure SSL protocol: PROTOCOL_TLSv1 + | +19 | func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 +20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 +21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 +22 | func(method=SSL.SSLv2_METHOD) # S502 +23 | func(method=SSL.SSLv23_METHOD) # S502 + | + +S502.py:22:6: S502 Call made with insecure SSL protocol: SSLv2_METHOD + | +20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 +21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 +22 | func(method=SSL.SSLv2_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^ S502 +23 | func(method=SSL.SSLv23_METHOD) # S502 +24 | func(method=SSL.SSLv3_METHOD) # S502 + | + +S502.py:23:6: S502 Call made with insecure SSL protocol: SSLv23_METHOD + | +21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 +22 | func(method=SSL.SSLv2_METHOD) # S502 +23 | func(method=SSL.SSLv23_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^^ S502 +24 | func(method=SSL.SSLv3_METHOD) # S502 +25 | func(method=SSL.TLSv1_METHOD) # S502 + | + +S502.py:24:6: S502 Call made with insecure SSL protocol: SSLv3_METHOD + | +22 | func(method=SSL.SSLv2_METHOD) # S502 +23 | func(method=SSL.SSLv23_METHOD) # S502 +24 | func(method=SSL.SSLv3_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^ S502 +25 | func(method=SSL.TLSv1_METHOD) # S502 + | + +S502.py:25:6: S502 Call made with insecure SSL protocol: TLSv1_METHOD + | +23 | func(method=SSL.SSLv23_METHOD) # S502 +24 | func(method=SSL.SSLv3_METHOD) # S502 +25 | func(method=SSL.TLSv1_METHOD) # S502 + | ^^^^^^^^^^^^^^^^^^^^^^^ S502 +26 | +27 | wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index c747417ed2656..4a2f88dd35372 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3512,6 +3512,7 @@ "S5", "S50", "S501", + "S502", "S504", "S505", "S506", From 06f2b88f6de010cb58019972b5c1ca6b49c83508 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Thu, 4 Jan 2024 12:56:38 +0100 Subject: [PATCH 4/5] Cargo fmt --- .../flake8_bandit/rules/ssl_insecure_version.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs index 09740c073a70c..9f300c8524cee 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs @@ -58,14 +58,12 @@ const INSECURE_SSL_PROTOCOLS: &[&str] = &[ /// S502 pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) { let keywords = match checker.semantic().resolve_call_path(call.func.as_ref()) { - Some(call_path) => { - match *call_path.as_slice() { - ["ssl", "wrap_socket"] => vec!["ssl_version"], - ["OpenSSL", "SSL", "Context"] => vec!["method"], - _ => vec!["ssl_version", "method"], - } + Some(call_path) => match *call_path.as_slice() { + ["ssl", "wrap_socket"] => vec!["ssl_version"], + ["OpenSSL", "SSL", "Context"] => vec!["method"], + _ => vec!["ssl_version", "method"], }, - None => vec!["ssl_version", "method"] + None => vec!["ssl_version", "method"], }; for arg in keywords { From b3c05fc2d6dadbec5618b513d00c9cf7c7a35505 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 4 Jan 2024 20:20:06 -0500 Subject: [PATCH 5/5] Reduce rule scope --- .../test/fixtures/flake8_bandit/S502.py | 17 --- .../rules/ssl_insecure_version.rs | 140 ++++++++---------- ...s__flake8_bandit__tests__S502_S502.py.snap | 82 ++-------- 3 files changed, 73 insertions(+), 166 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py index 1d0751b1f35e8..5d8118b15874e 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S502.py @@ -11,23 +11,6 @@ Context(method=SSL.SSLv3_METHOD) # S502 Context(method=SSL.TLSv1_METHOD) # S502 - -def func(): - pass - - -func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 -func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 -func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 -func(method=SSL.SSLv2_METHOD) # S502 -func(method=SSL.SSLv23_METHOD) # S502 -func(method=SSL.SSLv3_METHOD) # S502 -func(method=SSL.TLSv1_METHOD) # S502 - wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK SSL.Context(method=SSL.TLS_SERVER_METHOD) # OK func(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK - - - - diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs index 9f300c8524cee..d0b94e07dab33 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssl_insecure_version.rs @@ -1,22 +1,25 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr, ExprCall}; -use ruff_python_semantic::analyze::typing::find_assigned_value; +use ruff_text_size::Ranged; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for calls to Python methods with parameters that indicate the used broken SSL/TLS -/// protocol versions. +/// Checks for function calls with parameters that indicate the use of insecure +/// SSL and TLS protocol versions. /// /// ## Why is this bad? -/// Several highly publicized exploitable flaws have been discovered in all versions of SSL and -/// early versions of TLS. It is strongly recommended that use of the following known broken -/// protocol versions be avoided: -/// - SSL v2 -/// - SSL v3 -/// - TLS v1 -/// - TLS v1.1 +/// Several highly publicized exploitable flaws have been discovered in all +/// versions of SSL and early versions of TLS. The following versions are +/// considered insecure, and should be avoided: +/// - SSL v2 +/// - SSL v3 +/// - TLS v1 +/// - TLS v1.1 +/// +/// This method supports detection on the Python's built-in `ssl` module and +/// the `pyOpenSSL` module. /// /// ## Example /// ```python @@ -39,81 +42,66 @@ pub struct SslInsecureVersion { impl Violation for SslInsecureVersion { #[derive_message_formats] fn message(&self) -> String { - format!("Call made with insecure SSL protocol: {}", self.protocol) + let SslInsecureVersion { protocol } = self; + format!("Call made with insecure SSL protocol: `{protocol}`") } } -const INSECURE_SSL_PROTOCOLS: &[&str] = &[ - "PROTOCOL_SSLv2", - "PROTOCOL_SSLv3", - "PROTOCOL_TLSv1", - "PROTOCOL_TLSv1_1", - "SSLv2_METHOD", - "SSLv23_METHOD", - "SSLv3_METHOD", - "TLSv1_METHOD", - "TLSv1_1_METHOD", -]; - /// S502 pub(crate) fn ssl_insecure_version(checker: &mut Checker, call: &ExprCall) { - let keywords = match checker.semantic().resolve_call_path(call.func.as_ref()) { - Some(call_path) => match *call_path.as_slice() { - ["ssl", "wrap_socket"] => vec!["ssl_version"], - ["OpenSSL", "SSL", "Context"] => vec!["method"], - _ => vec!["ssl_version", "method"], - }, - None => vec!["ssl_version", "method"], + let Some(keyword) = checker + .semantic() + .resolve_call_path(call.func.as_ref()) + .and_then(|call_path| match call_path.as_slice() { + ["ssl", "wrap_socket"] => Some("ssl_version"), + ["OpenSSL", "SSL", "Context"] => Some("method"), + _ => None, + }) + else { + return; }; - for arg in keywords { - let Some(keyword) = call.arguments.find_keyword(arg) else { - continue; - }; - match &keyword.value { - Expr::Name(ast::ExprName { id, .. }) => { - let Some(val) = find_assigned_value(id, checker.semantic()) else { - continue; - }; - match val { - Expr::Name(ast::ExprName { id, .. }) => { - if INSECURE_SSL_PROTOCOLS.contains(&id.as_str()) { - checker.diagnostics.push(Diagnostic::new( - SslInsecureVersion { - protocol: id.to_string(), - }, - keyword.range, - )); - } - } - Expr::Attribute(ast::ExprAttribute { attr, .. }) => { - if INSECURE_SSL_PROTOCOLS.contains(&attr.as_str()) { - checker.diagnostics.push(Diagnostic::new( - SslInsecureVersion { - protocol: attr.to_string(), - }, - keyword.range, - )); - } - } - _ => { - continue; - } - } - } - Expr::Attribute(ast::ExprAttribute { attr, .. }) => { - if INSECURE_SSL_PROTOCOLS.contains(&attr.as_str()) { - checker.diagnostics.push(Diagnostic::new( - SslInsecureVersion { - protocol: attr.to_string(), - }, - keyword.range, - )); - } + let Some(keyword) = call.arguments.find_keyword(keyword) else { + return; + }; + + match &keyword.value { + Expr::Name(ast::ExprName { id, .. }) => { + if is_insecure_protocol(id) { + checker.diagnostics.push(Diagnostic::new( + SslInsecureVersion { + protocol: id.to_string(), + }, + keyword.range(), + )); } - _ => { - continue; + } + Expr::Attribute(ast::ExprAttribute { attr, .. }) => { + if is_insecure_protocol(attr) { + checker.diagnostics.push(Diagnostic::new( + SslInsecureVersion { + protocol: attr.to_string(), + }, + keyword.range(), + )); } } + _ => {} } } + +/// Returns `true` if the given protocol name is insecure. +fn is_insecure_protocol(name: &str) -> bool { + matches!( + name, + "PROTOCOL_SSLv2" + | "PROTOCOL_SSLv3" + | "PROTOCOL_TLSv1" + | "PROTOCOL_TLSv1_1" + | "SSLv2_METHOD" + | "SSLv23_METHOD" + | "SSLv3_METHOD" + | "TLSv1_METHOD" + | "TLSv1_1_METHOD" + ) +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap index d3aeb02c12d10..31a2a690416c5 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S502_S502.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S502.py:6:13: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv3 +S502.py:6:13: S502 Call made with insecure SSL protocol: `PROTOCOL_SSLv3` | 4 | from OpenSSL.SSL import Context 5 | @@ -11,7 +11,7 @@ S502.py:6:13: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv3 8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 | -S502.py:7:17: S502 Call made with insecure SSL protocol: PROTOCOL_TLSv1 +S502.py:7:17: S502 Call made with insecure SSL protocol: `PROTOCOL_TLSv1` | 6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 @@ -20,7 +20,7 @@ S502.py:7:17: S502 Call made with insecure SSL protocol: PROTOCOL_TLSv1 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 | -S502.py:8:17: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv2 +S502.py:8:17: S502 Call made with insecure SSL protocol: `PROTOCOL_SSLv2` | 6 | wrap_socket(ssl_version=ssl.PROTOCOL_SSLv3) # S502 7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 @@ -30,7 +30,7 @@ S502.py:8:17: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv2 10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 | -S502.py:9:13: S502 Call made with insecure SSL protocol: SSLv2_METHOD +S502.py:9:13: S502 Call made with insecure SSL protocol: `SSLv2_METHOD` | 7 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1) # S502 8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 @@ -40,7 +40,7 @@ S502.py:9:13: S502 Call made with insecure SSL protocol: SSLv2_METHOD 11 | Context(method=SSL.SSLv3_METHOD) # S502 | -S502.py:10:13: S502 Call made with insecure SSL protocol: SSLv23_METHOD +S502.py:10:13: S502 Call made with insecure SSL protocol: `SSLv23_METHOD` | 8 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_SSLv2) # S502 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 @@ -50,7 +50,7 @@ S502.py:10:13: S502 Call made with insecure SSL protocol: SSLv23_METHOD 12 | Context(method=SSL.TLSv1_METHOD) # S502 | -S502.py:11:9: S502 Call made with insecure SSL protocol: SSLv3_METHOD +S502.py:11:9: S502 Call made with insecure SSL protocol: `SSLv3_METHOD` | 9 | SSL.Context(method=SSL.SSLv2_METHOD) # S502 10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 @@ -59,78 +59,14 @@ S502.py:11:9: S502 Call made with insecure SSL protocol: SSLv3_METHOD 12 | Context(method=SSL.TLSv1_METHOD) # S502 | -S502.py:12:9: S502 Call made with insecure SSL protocol: TLSv1_METHOD +S502.py:12:9: S502 Call made with insecure SSL protocol: `TLSv1_METHOD` | 10 | SSL.Context(method=SSL.SSLv23_METHOD) # S502 11 | Context(method=SSL.SSLv3_METHOD) # S502 12 | Context(method=SSL.TLSv1_METHOD) # S502 | ^^^^^^^^^^^^^^^^^^^^^^^ S502 - | - -S502.py:19:6: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv2 - | -19 | func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 -20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 -21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 - | - -S502.py:20:6: S502 Call made with insecure SSL protocol: PROTOCOL_SSLv3 - | -19 | func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 -20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 -21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 -22 | func(method=SSL.SSLv2_METHOD) # S502 - | - -S502.py:21:6: S502 Call made with insecure SSL protocol: PROTOCOL_TLSv1 - | -19 | func(ssl_version=ssl.PROTOCOL_SSLv2) # S502 -20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 -21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S502 -22 | func(method=SSL.SSLv2_METHOD) # S502 -23 | func(method=SSL.SSLv23_METHOD) # S502 - | - -S502.py:22:6: S502 Call made with insecure SSL protocol: SSLv2_METHOD - | -20 | func(ssl_version=ssl.PROTOCOL_SSLv3) # S502 -21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 -22 | func(method=SSL.SSLv2_METHOD) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^ S502 -23 | func(method=SSL.SSLv23_METHOD) # S502 -24 | func(method=SSL.SSLv3_METHOD) # S502 - | - -S502.py:23:6: S502 Call made with insecure SSL protocol: SSLv23_METHOD - | -21 | func(ssl_version=ssl.PROTOCOL_TLSv1) # S502 -22 | func(method=SSL.SSLv2_METHOD) # S502 -23 | func(method=SSL.SSLv23_METHOD) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^^ S502 -24 | func(method=SSL.SSLv3_METHOD) # S502 -25 | func(method=SSL.TLSv1_METHOD) # S502 - | - -S502.py:24:6: S502 Call made with insecure SSL protocol: SSLv3_METHOD - | -22 | func(method=SSL.SSLv2_METHOD) # S502 -23 | func(method=SSL.SSLv23_METHOD) # S502 -24 | func(method=SSL.SSLv3_METHOD) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^ S502 -25 | func(method=SSL.TLSv1_METHOD) # S502 - | - -S502.py:25:6: S502 Call made with insecure SSL protocol: TLSv1_METHOD - | -23 | func(method=SSL.SSLv23_METHOD) # S502 -24 | func(method=SSL.SSLv3_METHOD) # S502 -25 | func(method=SSL.TLSv1_METHOD) # S502 - | ^^^^^^^^^^^^^^^^^^^^^^^ S502 -26 | -27 | wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK +13 | +14 | wrap_socket(ssl_version=ssl.PROTOCOL_TLS_CLIENT) # OK |