Skip to content

Commit

Permalink
Add remaining checks, refine tests based on bandit spec
Browse files Browse the repository at this point in the history
  • Loading branch information
qdegraaf committed Jan 2, 2024
1 parent 8f0fae5 commit 5292b86
Show file tree
Hide file tree
Showing 15 changed files with 363 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from xml import etree # S405
import xml.etree as xmle # S405
import xml.etree # S405
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
@@ -1,4 +1,2 @@
from xml.dom import expatbuilder # S407
from xml.sax import expatreader # S407
import xml.dom.expatbuilder # S407
import xml.sax.expatreader # S407
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
import wsgiref.handlers # S412
from twisted.internet import reactor # S412
from twisted.web import static, server, twcgi # S412
from twisted.web.twcgi import CGIScript # S412
10 changes: 4 additions & 6 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S413.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
from cryptography.hazmat import backends # S413
from cryptography.hazmat.primitives.asymmetric import dsa # S413
from cryptography.hazmat.primitives.asymmetric import ec # S413
from cryptography.hazmat.primitives.asymmetric import rsa # S413
from Crypto.PublicKey import RSA as pycrypto_rsa # S413
from Cryptodome.PublicKey import DSA as pycryptodomex_dsa # S413
import Crypto.Hash # S413
from Crypto.Hash import MD2 # S413
import Crypto.PublicKey # S413
from Crypto.PublicKey import RSA # S413
18 changes: 16 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,15 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
Rule::SuspiciousFtplibImport,
Rule::SuspiciousPickleImport,
Rule::SuspiciousSubprocessImport,
Rule::SuspiciousXmlEtreeImport,
Rule::SuspiciousXmlSaxImport,
Rule::SuspiciousXmlExpatImport,
Rule::SuspiciousXmlMinidomImport,
Rule::SuspiciousXmlPulldomImport,
Rule::SuspiciousLxmlImport,
Rule::SuspiciousXmlrpclibImport,
Rule::SuspiciousXmlrpcImport,
Rule::SuspiciousHttpoxyImport,
Rule::SuspiciousPycryptoImport,
Rule::SuspiciousPyghmiImport,
]) {
flake8_bandit::rules::suspicious_imports(checker, stmt);
Expand Down Expand Up @@ -767,8 +774,15 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
Rule::SuspiciousFtplibImport,
Rule::SuspiciousPickleImport,
Rule::SuspiciousSubprocessImport,
Rule::SuspiciousXmlEtreeImport,
Rule::SuspiciousXmlSaxImport,
Rule::SuspiciousXmlExpatImport,
Rule::SuspiciousXmlMinidomImport,
Rule::SuspiciousXmlPulldomImport,
Rule::SuspiciousLxmlImport,
Rule::SuspiciousXmlrpclibImport,
Rule::SuspiciousXmlrpcImport,
Rule::SuspiciousHttpoxyImport,
Rule::SuspiciousPycryptoImport,
Rule::SuspiciousPyghmiImport,
]) {
flake8_bandit::rules::suspicious_imports(checker, stmt);
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "408") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousXmlMinidomImport),
(Flake8Bandit, "409") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousXmlPulldomImport),
(Flake8Bandit, "410") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousLxmlImport),
(Flake8Bandit, "411") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousXmlrpclibImport),
(Flake8Bandit, "411") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousXmlrpcImport),
(Flake8Bandit, "412") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousHttpoxyImport),
(Flake8Bandit, "413") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousPycryptoImport),
(Flake8Bandit, "415") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousPyghmiImport),
Expand Down
9 changes: 8 additions & 1 deletion crates/ruff_linter/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,15 @@ mod tests {
#[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::SuspiciousXmlrpclibImport, Path::new("S411.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"))]
Expand Down
167 changes: 161 additions & 6 deletions crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ impl Violation for SuspiciousLxmlImport {
}

#[violation]
pub struct SuspiciousXmlrpclibImport;
pub struct SuspiciousXmlrpcImport;

impl Violation for SuspiciousXmlrpclibImport {
impl Violation for SuspiciousXmlrpcImport {
#[derive_message_formats]
fn message(&self) -> String {
format!("XMLRPC is particularly dangerous as it is also concerned with communicating data over a network")
Expand Down Expand Up @@ -153,7 +153,7 @@ impl Violation for SuspiciousPyghmiImport {
}
}

/// S401, S402, S403, S404, S405, S406, S407, S408, S409, S410, S411, S412, S413
/// S401, S402, S403, S404, S405, S406, S407, S408, S409, S410, S411, S412, S413, S415
pub(crate) fn suspicious_imports(checker: &mut Checker, stmt: &Stmt) {
match stmt {
Stmt::Import(ast::StmtImport { names, .. }) => {
Expand All @@ -179,16 +179,51 @@ pub(crate) fn suspicious_imports(checker: &mut Checker, stmt: &Stmt) {
DiagnosticKind::from(SuspiciousSubprocessImport),
name.range,
),
"xml.etree.cElementTree" | "xml.etree.ElementTree" => {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlEtreeImport),
name.range,
);
}
"xml.sax" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlSaxImport),
name.range,
),
"xml.dom.expatbuilder" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlExpatImport),
name.range,
),
"xml.dom.minidom" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlMinidomImport),
name.range,
),
"xml.dom.pulldom" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlPulldomImport),
name.range,
),
"lxml" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousLxmlImport),
name.range,
),
"xmlrpc" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlrpclibImport),
DiagnosticKind::from(SuspiciousXmlrpcImport),
name.range,
),
"Crypto.Cipher" | "Crypto.Hash" | "Crypto.IO" | "Crypto.Protocol"
| "Crypto.PublicKey" | "Crypto.Random" | "Crypto.Signature" | "Crypto.Util" => {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousPycryptoImport),
name.range,
);
}
"pyghmi" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousPyghmiImport),
Expand All @@ -198,7 +233,7 @@ pub(crate) fn suspicious_imports(checker: &mut Checker, stmt: &Stmt) {
}
}
}
Stmt::ImportFrom(ast::StmtImportFrom { module, .. }) => {
Stmt::ImportFrom(ast::StmtImportFrom { module, names, .. }) => {
let Some(identifier) = module else { return };
match identifier.as_str() {
"telnetlib" => check_and_push_diagnostic(
Expand All @@ -221,16 +256,136 @@ pub(crate) fn suspicious_imports(checker: &mut Checker, stmt: &Stmt) {
DiagnosticKind::from(SuspiciousSubprocessImport),
identifier.range(),
),
"xml.etree" => {
for name in names {
if &name.name == "cElementTree" || &name.name == "ElementTree" {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlEtreeImport),
identifier.range(),
);
}
}
}
"xml.etree.cElementTree" | "xml.etree.ElementTree" => {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlEtreeImport),
identifier.range(),
);
}
"xml" => {
for name in names {
if &name.name == "sax" {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlSaxImport),
identifier.range(),
);
}
}
}
"xml.sax" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlSaxImport),
identifier.range(),
),
"xml.dom" => {
for name in names {
match name.name.as_str() {
"expatbuilder" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlExpatImport),
identifier.range(),
),
"minidom" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlMinidomImport),
identifier.range(),
),
"pulldom" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlPulldomImport),
identifier.range(),
),
_ => (),
}
}
}
"xml.dom.expatbuilder" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlExpatImport),
identifier.range(),
),
"xml.dom.minidom" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlMinidomImport),
identifier.range(),
),
"xml.dom.pulldom" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlPulldomImport),
identifier.range(),
),
"lxml" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousLxmlImport),
identifier.range(),
),
"xmlrpc" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousXmlrpclibImport),
DiagnosticKind::from(SuspiciousXmlrpcImport),
identifier.range(),
),
"wsgiref.handlers" => {
for name in names {
if &name.name == "CGIHandler" {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousHttpoxyImport),
identifier.range(),
);
}
}
}
"twisted.web.twcgi" => {
for name in names {
if &name.name == "CGIScript" {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousHttpoxyImport),
identifier.range(),
);
}
}
}
"Crypto" => {
for name in names {
if &name.name == "Cipher"
|| &name.name == "Hash"
|| &name.name == "IO"
|| &name.name == "Protocol"
|| &name.name == "PublicKey"
|| &name.name == "Random"
|| &name.name == "Signature"
|| &name.name == "Util"
{
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousPycryptoImport),
identifier.range(),
);
}
}
}
"Crypto.Cipher" | "Crypto.Hash" | "Crypto.IO" | "Crypto.Protocol"
| "Crypto.PublicKey" | "Crypto.Random" | "Crypto.Signature" | "Crypto.Util" => {
check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousPycryptoImport),
identifier.range(),
);
}
"pyghmi" => check_and_push_diagnostic(
checker,
DiagnosticKind::from(SuspiciousPyghmiImport),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S405.py:1:8: S405 `xml.etree` methods are vulnerable to XML attacks
|
1 | import xml.etree.cElementTree # S405
| ^^^^^^^^^^^^^^^^^^^^^^ S405
2 | from xml.etree import cElementTree # S405
3 | import xml.etree.ElementTree # S405
|

S405.py:2:6: S405 `xml.etree` methods are vulnerable to XML attacks
|
1 | import xml.etree.cElementTree # S405
2 | from xml.etree import cElementTree # S405
| ^^^^^^^^^ S405
3 | import xml.etree.ElementTree # S405
4 | from xml.etree import ElementTree # S405
|

S405.py:3:8: S405 `xml.etree` methods are vulnerable to XML attacks
|
1 | import xml.etree.cElementTree # S405
2 | from xml.etree import cElementTree # S405
3 | import xml.etree.ElementTree # S405
| ^^^^^^^^^^^^^^^^^^^^^ S405
4 | from xml.etree import ElementTree # S405
|

S405.py:4:6: S405 `xml.etree` methods are vulnerable to XML attacks
|
2 | from xml.etree import cElementTree # S405
3 | import xml.etree.ElementTree # S405
4 | from xml.etree import ElementTree # S405
| ^^^^^^^^^ S405
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S406.py:1:6: S406 `xml.sax` methods are vulnerable to XML attacks
|
1 | from xml import sax # S406
| ^^^ S406
2 | import xml.sax as xmls # S406
3 | import xml.sax # S406
|

S406.py:2:8: S406 `xml.sax` methods are vulnerable to XML attacks
|
1 | from xml import sax # S406
2 | import xml.sax as xmls # S406
| ^^^^^^^^^^^^^^^ S406
3 | import xml.sax # S406
|

S406.py:3:8: S406 `xml.sax` methods are vulnerable to XML attacks
|
1 | from xml import sax # S406
2 | import xml.sax as xmls # S406
3 | import xml.sax # S406
| ^^^^^^^ S406
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S407.py:1:6: S407 `xml.dom.expatbuilder` is vulnerable to XML attacks
|
1 | from xml.dom import expatbuilder # S407
| ^^^^^^^ S407
2 | import xml.dom.expatbuilder # S407
|

S407.py:2:8: S407 `xml.dom.expatbuilder` is vulnerable to XML attacks
|
1 | from xml.dom import expatbuilder # S407
2 | import xml.dom.expatbuilder # S407
| ^^^^^^^^^^^^^^^^^^^^ S407
|


Loading

0 comments on commit 5292b86

Please sign in to comment.