Skip to content

Commit

Permalink
[flake8-bandit] Implement S4XX suspicious import rules (#8831)
Browse files Browse the repository at this point in the history
## Summary

Adds all `S4XX` rules to the
[flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin
port.

There is a lot of documentation to write, some tests can be expanded and
implementation can probably be refactored to be more compact. As there
is some discussion on whether this is actually useful. (See:
#1646 (comment)),
wanted to check which rules we want to have before I go through the
process of polishing this up.

## Test Plan

Fixtures for all rules based on `flake8-bandit`
[tests](https://github.com/tylerwince/flake8-bandit/tree/main/tests)

## Issue link

Refers: #1646
  • Loading branch information
qdegraaf authored Jan 3, 2024
1 parent e3ad163 commit 5c93a52
Show file tree
Hide file tree
Showing 34 changed files with 1,079 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import telnetlib # S401
from telnetlib import Telnet # S401
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import ftplib # S402
from ftplib import FTP # S402
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import dill # S403
from dill import objects # S403
import shelve
from shelve import open
import cPickle
from cPickle import load
import pickle
from pickle import load
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import subprocess # S404
from subprocess import Popen # S404
from subprocess import Popen as pop # S404
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import xml.etree.cElementTree # S405
from xml.etree import cElementTree # S405
import xml.etree.ElementTree # S405
from xml.etree import ElementTree # S405
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from xml import sax # S406
import xml.sax as xmls # S406
import xml.sax # S406
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from xml.dom import expatbuilder # S407
import xml.dom.expatbuilder # S407
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from xml.dom.minidom import parseString # S408
import xml.dom.minidom # S408
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from xml.dom.pulldom import parseString # S409
import xml.dom.pulldom # S409
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import lxml # S410
from lxml import etree # S410
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import xmlrpc # S411
from xmlrpc import server # S411
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from twisted.web.twcgi import CGIScript # S412
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Crypto.Hash # S413
from Crypto.Hash import MD2 # S413
import Crypto.PublicKey # S413
from Crypto.PublicKey import RSA # S413
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import pyghmi # S415
from pyghmi import foo # S415

36 changes: 36 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,24 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::DeprecatedMockImport) {
pyupgrade::rules::deprecated_mock_import(checker, stmt);
}
if checker.any_enabled(&[
Rule::SuspiciousTelnetlibImport,
Rule::SuspiciousFtplibImport,
Rule::SuspiciousPickleImport,
Rule::SuspiciousSubprocessImport,
Rule::SuspiciousXmlEtreeImport,
Rule::SuspiciousXmlSaxImport,
Rule::SuspiciousXmlExpatImport,
Rule::SuspiciousXmlMinidomImport,
Rule::SuspiciousXmlPulldomImport,
Rule::SuspiciousLxmlImport,
Rule::SuspiciousXmlrpcImport,
Rule::SuspiciousHttpoxyImport,
Rule::SuspiciousPycryptoImport,
Rule::SuspiciousPyghmiImport,
]) {
flake8_bandit::rules::suspicious_imports(checker, stmt);
}

for alias in names {
if checker.enabled(Rule::NonAsciiImportName) {
Expand Down Expand Up @@ -751,6 +769,24 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pyupgrade::rules::unnecessary_builtin_import(checker, stmt, module, names);
}
}
if checker.any_enabled(&[
Rule::SuspiciousTelnetlibImport,
Rule::SuspiciousFtplibImport,
Rule::SuspiciousPickleImport,
Rule::SuspiciousSubprocessImport,
Rule::SuspiciousXmlEtreeImport,
Rule::SuspiciousXmlSaxImport,
Rule::SuspiciousXmlExpatImport,
Rule::SuspiciousXmlMinidomImport,
Rule::SuspiciousXmlPulldomImport,
Rule::SuspiciousLxmlImport,
Rule::SuspiciousXmlrpcImport,
Rule::SuspiciousHttpoxyImport,
Rule::SuspiciousPycryptoImport,
Rule::SuspiciousPyghmiImport,
]) {
flake8_bandit::rules::suspicious_imports(checker, stmt);
}
if checker.enabled(Rule::BannedApi) {
if let Some(module) =
helpers::resolve_imported_module_path(level, module, checker.module_path)
Expand Down
14 changes: 14 additions & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,20 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "321") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousFTPLibUsage),
(Flake8Bandit, "323") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousUnverifiedContextUsage),
(Flake8Bandit, "324") => (RuleGroup::Stable, rules::flake8_bandit::rules::HashlibInsecureHashFunction),
(Flake8Bandit, "401") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousTelnetlibImport),
(Flake8Bandit, "402") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousFtplibImport),
(Flake8Bandit, "403") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousPickleImport),
(Flake8Bandit, "404") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousSubprocessImport),
(Flake8Bandit, "405") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousXmlEtreeImport),
(Flake8Bandit, "406") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousXmlSaxImport),
(Flake8Bandit, "407") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousXmlExpatImport),
(Flake8Bandit, "408") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousXmlMinidomImport),
(Flake8Bandit, "409") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousXmlPulldomImport),
(Flake8Bandit, "410") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousLxmlImport),
(Flake8Bandit, "411") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousXmlrpcImport),
(Flake8Bandit, "412") => (RuleGroup::Preview, rules::flake8_bandit::rules::SuspiciousHttpoxyImport),
(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, "505") => (RuleGroup::Preview, rules::flake8_bandit::rules::WeakCryptographicKey),
(Flake8Bandit, "506") => (RuleGroup::Stable, rules::flake8_bandit::rules::UnsafeYAMLLoad),
Expand Down
14 changes: 14 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ mod tests {
#[test_case(Rule::SuspiciousEvalUsage, Path::new("S307.py"))]
#[test_case(Rule::SuspiciousURLOpenUsage, Path::new("S310.py"))]
#[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
#[test_case(Rule::SuspiciousTelnetlibImport, Path::new("S401.py"))]
#[test_case(Rule::SuspiciousFtplibImport, Path::new("S402.py"))]
#[test_case(Rule::SuspiciousPickleImport, Path::new("S403.py"))]
#[test_case(Rule::SuspiciousSubprocessImport, Path::new("S404.py"))]
#[test_case(Rule::SuspiciousXmlEtreeImport, Path::new("S405.py"))]
#[test_case(Rule::SuspiciousXmlSaxImport, Path::new("S406.py"))]
#[test_case(Rule::SuspiciousXmlExpatImport, Path::new("S407.py"))]
#[test_case(Rule::SuspiciousXmlMinidomImport, Path::new("S408.py"))]
#[test_case(Rule::SuspiciousXmlPulldomImport, Path::new("S409.py"))]
#[test_case(Rule::SuspiciousLxmlImport, Path::new("S410.py"))]
#[test_case(Rule::SuspiciousXmlrpcImport, Path::new("S411.py"))]
#[test_case(Rule::SuspiciousHttpoxyImport, Path::new("S412.py"))]
#[test_case(Rule::SuspiciousPycryptoImport, Path::new("S413.py"))]
#[test_case(Rule::SuspiciousPyghmiImport, Path::new("S415.py"))]
#[test_case(Rule::TryExceptContinue, Path::new("S112.py"))]
#[test_case(Rule::TryExceptPass, Path::new("S110.py"))]
#[test_case(Rule::UnixCommandWildcardInjection, Path::new("S609.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 @@ -21,6 +21,7 @@ pub(crate) use snmp_insecure_version::*;
pub(crate) use snmp_weak_cryptography::*;
pub(crate) use ssh_no_host_key_verification::*;
pub(crate) use suspicious_function_call::*;
pub(crate) use suspicious_imports::*;
pub(crate) use tarfile_unsafe_members::*;
pub(crate) use try_except_continue::*;
pub(crate) use try_except_pass::*;
Expand Down Expand Up @@ -50,6 +51,7 @@ mod snmp_insecure_version;
mod snmp_weak_cryptography;
mod ssh_no_host_key_verification;
mod suspicious_function_call;
mod suspicious_imports;
mod tarfile_unsafe_members;
mod try_except_continue;
mod try_except_pass;
Expand Down
Loading

0 comments on commit 5c93a52

Please sign in to comment.