Skip to content

Commit

Permalink
[flake8-bandit] Add S504 SslWithNoVersion rule (#9384)
Browse files Browse the repository at this point in the history
## Summary
Adds `S504` rule for the
[flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin
port.

Checks for calls to `ssl.wrap_socket` which have no `ssl_version`
argument set. See also
https://bandit.readthedocs.io/en/latest/_modules/bandit/plugins/insecure_ssl_tls.html#ssl_with_no_version

## Test Plan

Fixture added 

## Issue Link

Refers: #1646
  • Loading branch information
qdegraaf authored Jan 3, 2024
1 parent 5c93a52 commit 3b323a0
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 2 deletions.
15 changes: 15 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S504.py
Original file line number Diff line number Diff line change
@@ -0,0 +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 Class:
def wrap_socket(self):
pass


obj = Class()
obj.wrap_socket() # OK
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(&[
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/registry/rule_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
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()` without an `ssl_version`.
///
/// ## Why is this bad?
/// This method is known to provide a default value that maximizes
/// compatibility, but permits use of insecure protocols.
///
/// ## 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 without an `ssl_version``")
}
}

/// 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"]))
{
if call.arguments.find_keyword("ssl_version").is_none() {
checker
.diagnostics
.push(Diagnostic::new(SslWithNoVersion, call.range()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S504.py:4:1: S504 `ssl.wrap_socket` called without an `ssl_version``
|
2 | from ssl import wrap_socket
3 |
4 | ssl.wrap_socket() # S504
| ^^^^^^^^^^^^^^^^^ S504
5 | wrap_socket() # S504
6 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK
|
S504.py:5:1: S504 `ssl.wrap_socket` called without an `ssl_version``
|
4 | ssl.wrap_socket() # S504
5 | wrap_socket() # S504
| ^^^^^^^^^^^^^ S504
6 | ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2) # OK
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3b323a0

Please sign in to comment.