[`flake8-bandit`] Implement `S503` `SslWithBadDefaults` rule (#9391)

## Summary

Adds S503 rule for the
[flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin
port.

Checks for function defs argument defaults which have an insecure
ssl_version value. See also
https://bandit.readthedocs.io/en/latest/_modules/bandit/plugins/insecure_ssl_tls.html#ssl_with_bad_defaults

Some logic and the `const` can be shared with
https://github.com/astral-sh/ruff/pull/9390. When one of the two is
merged.

## Test Plan

Fixture added

## Issue Link

Refers: https://github.com/astral-sh/ruff/issues/1646
This commit is contained in:
qdegraaf 2024-01-05 02:38:41 +01:00 committed by GitHub
parent 6dfc1ccd6f
commit c11f65381f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 169 additions and 0 deletions

View File

@ -0,0 +1,23 @@
import ssl
from OpenSSL import SSL
from ssl import PROTOCOL_TLSv1
def func(version=ssl.PROTOCOL_SSLv2): # S503
pass
def func(protocol=SSL.SSLv2_METHOD): # S503
pass
def func(version=SSL.SSLv23_METHOD): # S503
pass
def func(protocol=PROTOCOL_TLSv1): # S503
pass
def func(version=SSL.TLSv1_2_METHOD): # OK
pass

View File

@ -374,6 +374,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ReimplementedOperator) { if checker.enabled(Rule::ReimplementedOperator) {
refurb::rules::reimplemented_operator(checker, &function_def.into()); refurb::rules::reimplemented_operator(checker, &function_def.into());
} }
if checker.enabled(Rule::SslWithBadDefaults) {
flake8_bandit::rules::ssl_with_bad_defaults(checker, function_def);
}
} }
Stmt::Return(_) => { Stmt::Return(_) => {
if checker.enabled(Rule::ReturnOutsideFunction) { if checker.enabled(Rule::ReturnOutsideFunction) {

View File

@ -643,6 +643,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "415") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPyghmiImport), (Flake8Bandit, "415") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPyghmiImport),
(Flake8Bandit, "501") => (RuleGroup::Stable, rules::flake8_bandit::rules::RequestWithNoCertValidation), (Flake8Bandit, "501") => (RuleGroup::Stable, rules::flake8_bandit::rules::RequestWithNoCertValidation),
(Flake8Bandit, "502") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslInsecureVersion), (Flake8Bandit, "502") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslInsecureVersion),
(Flake8Bandit, "503") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslWithBadDefaults),
(Flake8Bandit, "504") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslWithNoVersion), (Flake8Bandit, "504") => (RuleGroup::Preview, rules::flake8_bandit::rules::SslWithNoVersion),
(Flake8Bandit, "505") => (RuleGroup::Preview, rules::flake8_bandit::rules::WeakCryptographicKey), (Flake8Bandit, "505") => (RuleGroup::Preview, rules::flake8_bandit::rules::WeakCryptographicKey),
(Flake8Bandit, "506") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnsafeYAMLLoad), (Flake8Bandit, "506") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnsafeYAMLLoad),

View File

@ -37,6 +37,7 @@ mod tests {
#[test_case(Rule::SnmpInsecureVersion, Path::new("S508.py"))] #[test_case(Rule::SnmpInsecureVersion, Path::new("S508.py"))]
#[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"))] #[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"))]
#[test_case(Rule::SslInsecureVersion, Path::new("S502.py"))] #[test_case(Rule::SslInsecureVersion, Path::new("S502.py"))]
#[test_case(Rule::SslWithBadDefaults, Path::new("S503.py"))]
#[test_case(Rule::SslWithNoVersion, Path::new("S504.py"))] #[test_case(Rule::SslWithNoVersion, Path::new("S504.py"))]
#[test_case(Rule::StartProcessWithAShell, Path::new("S605.py"))] #[test_case(Rule::StartProcessWithAShell, Path::new("S605.py"))]
#[test_case(Rule::StartProcessWithNoShell, Path::new("S606.py"))] #[test_case(Rule::StartProcessWithNoShell, Path::new("S606.py"))]

View File

@ -21,6 +21,7 @@ pub(crate) use snmp_insecure_version::*;
pub(crate) use snmp_weak_cryptography::*; pub(crate) use snmp_weak_cryptography::*;
pub(crate) use ssh_no_host_key_verification::*; pub(crate) use ssh_no_host_key_verification::*;
pub(crate) use ssl_insecure_version::*; pub(crate) use ssl_insecure_version::*;
pub(crate) use ssl_with_bad_defaults::*;
pub(crate) use ssl_with_no_version::*; pub(crate) use ssl_with_no_version::*;
pub(crate) use suspicious_function_call::*; pub(crate) use suspicious_function_call::*;
pub(crate) use suspicious_imports::*; pub(crate) use suspicious_imports::*;
@ -53,6 +54,7 @@ mod snmp_insecure_version;
mod snmp_weak_cryptography; mod snmp_weak_cryptography;
mod ssh_no_host_key_verification; mod ssh_no_host_key_verification;
mod ssl_insecure_version; mod ssl_insecure_version;
mod ssl_with_bad_defaults;
mod ssl_with_no_version; mod ssl_with_no_version;
mod suspicious_function_call; mod suspicious_function_call;
mod suspicious_imports; mod suspicious_imports;

View File

@ -0,0 +1,106 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, StmtFunctionDef};
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for function definitions with default arguments set to 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. The following versions are
/// considered insecure, and should be avoided:
/// - SSL v2
/// - SSL v3
/// - TLS v1
/// - TLS v1.1
///
/// ## Example
/// ```python
/// import ssl
///
///
/// def func(version=ssl.PROTOCOL_TLSv1):
/// ...
/// ```
///
/// Use instead:
/// ```python
/// import ssl
///
///
/// def func(version=ssl.PROTOCOL_TLSv1_2):
/// ...
/// ```
#[violation]
pub struct SslWithBadDefaults {
protocol: String,
}
impl Violation for SslWithBadDefaults {
#[derive_message_formats]
fn message(&self) -> String {
let SslWithBadDefaults { protocol } = self;
format!("Argument default set to insecure SSL protocol: `{protocol}`")
}
}
/// S503
pub(crate) fn ssl_with_bad_defaults(checker: &mut Checker, function_def: &StmtFunctionDef) {
function_def
.parameters
.posonlyargs
.iter()
.chain(
function_def
.parameters
.args
.iter()
.chain(function_def.parameters.kwonlyargs.iter()),
)
.for_each(|param| {
if let Some(default) = &param.default {
match default.as_ref() {
Expr::Name(ast::ExprName { id, range, .. }) => {
if is_insecure_protocol(id.as_str()) {
checker.diagnostics.push(Diagnostic::new(
SslWithBadDefaults {
protocol: id.to_string(),
},
*range,
));
}
}
Expr::Attribute(ast::ExprAttribute { attr, range, .. }) => {
if is_insecure_protocol(attr.as_str()) {
checker.diagnostics.push(Diagnostic::new(
SslWithBadDefaults {
protocol: attr.to_string(),
},
*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"
)
}

View File

@ -0,0 +1,32 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S503.py:6:18: S503 Argument default set to insecure SSL protocol: `PROTOCOL_SSLv2`
|
6 | def func(version=ssl.PROTOCOL_SSLv2): # S503
| ^^^^^^^^^^^^^^^^^^ S503
7 | pass
|
S503.py:10:19: S503 Argument default set to insecure SSL protocol: `SSLv2_METHOD`
|
10 | def func(protocol=SSL.SSLv2_METHOD): # S503
| ^^^^^^^^^^^^^^^^ S503
11 | pass
|
S503.py:14:18: S503 Argument default set to insecure SSL protocol: `SSLv23_METHOD`
|
14 | def func(version=SSL.SSLv23_METHOD): # S503
| ^^^^^^^^^^^^^^^^^ S503
15 | pass
|
S503.py:18:19: S503 Argument default set to insecure SSL protocol: `PROTOCOL_TLSv1`
|
18 | def func(protocol=PROTOCOL_TLSv1): # S503
| ^^^^^^^^^^^^^^ S503
19 | pass
|

1
ruff.schema.json generated
View File

@ -3513,6 +3513,7 @@
"S50", "S50",
"S501", "S501",
"S502", "S502",
"S503",
"S504", "S504",
"S505", "S505",
"S506", "S506",