Updated output format

This commit is contained in:
colin99d 2023-03-06 09:41:07 -05:00 committed by Charlie Marsh
parent b0242ccfa1
commit 6c2d430430
12 changed files with 324 additions and 266 deletions

View File

@ -1,6 +1,6 @@
from ast import literal_eval
eval("3 + 4") # noqa: S001
eval("3 + 4")
literal_eval({1: 2})

View File

@ -2445,8 +2445,8 @@ where
}
// flake8-bandit
if self.settings.rules.enabled(Rule::DenylistCall) {
flake8_bandit::rules::denylist_calls(self, expr);
if self.settings.rules.enabled(Rule::DeniedFunctionCall) {
flake8_bandit::rules::denied_function_call(self, expr);
}
// flake8-bugbear

View File

@ -463,7 +463,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Eradicate, "001") => Rule::CommentedOutCode,
// flake8-bandit
(Flake8Bandit, "001") => Rule::DenylistCall,
(Flake8Bandit, "001") => Rule::DeniedFunctionCall,
(Flake8Bandit, "101") => Rule::Assert,
(Flake8Bandit, "102") => Rule::ExecBuiltin,
(Flake8Bandit, "103") => Rule::BadFilePermissions,

View File

@ -429,7 +429,7 @@ ruff_macros::register_rules!(
rules::eradicate::rules::CommentedOutCode,
// flake8-bandit
rules::flake8_bandit::rules::Assert,
rules::flake8_bandit::rules::DenylistCall,
rules::flake8_bandit::rules::DeniedFunctionCall,
rules::flake8_bandit::rules::ExecBuiltin,
rules::flake8_bandit::rules::BadFilePermissions,
rules::flake8_bandit::rules::HardcodedBindAllInterfaces,

View File

@ -1,9 +1,6 @@
use once_cell::sync::Lazy;
use regex::Regex;
use ruff_macros::CacheKey;
use rustpython_parser::ast::{Constant, Expr, ExprKind};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use crate::checkers::ast::Checker;
@ -48,13 +45,3 @@ pub fn is_untyped_exception(type_: Option<&Expr>, checker: &Checker) -> bool {
}
})
}
#[derive(
Debug, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema, Hash, CacheKey, PartialOrd,
)]
pub enum Severity {
#[default]
Low,
Medium,
High,
}

View File

@ -12,11 +12,11 @@ mod tests {
use test_case::test_case;
use crate::registry::Rule;
use crate::rules::flake8_bandit::helpers::Severity;
use crate::rules::flake8_bandit::settings::Severity;
use crate::settings::Settings;
use crate::test::test_path;
#[test_case(Rule::DenylistCall, Path::new("S001.py"); "S001")]
#[test_case(Rule::DeniedFunctionCall, Path::new("S001.py"); "S001")]
#[test_case(Rule::Assert, Path::new("S101.py"); "S101")]
#[test_case(Rule::ExecBuiltin, Path::new("S102.py"); "S102")]
#[test_case(Rule::BadFilePermissions, Path::new("S103.py"); "S103")]

View File

@ -0,0 +1,294 @@
//! Check for calls to suspicious functions, or calls into suspicious modules.
//!
//! See: <https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html>
use rustpython_parser::ast::{Expr, ExprKind};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;
use crate::checkers::ast::Checker;
use crate::rules::flake8_bandit::settings::Severity;
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum Reason {
Pickle,
Marshal,
InsecureHash,
InsecureCipher,
Mktemp,
Eval,
MarkSafe,
URLOpen,
NonCryptographicRandom,
UntrustedXML,
UnverifiedSSL,
Telnet,
FTPLib,
}
#[violation]
pub struct DeniedFunctionCall {
pub reason: Reason,
}
impl Violation for DeniedFunctionCall {
#[derive_message_formats]
fn message(&self) -> String {
let DeniedFunctionCall { reason } = self;
match reason {
Reason::Pickle => format!("`pickle` and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue"),
Reason::Marshal => format!("Deserialization with the `marshal` module is possibly dangerous"),
Reason::InsecureHash => format!("Use of insecure MD2, MD4, MD5, or SHA1 hash function"),
Reason::InsecureCipher => format!("Use of insecure cipher or cipher mode, replace with a known secure cipher such as AES"),
Reason::Mktemp => format!("Use of insecure and deprecated function (`mktemp`)"),
Reason::Eval => format!("Use of possibly insecure function; consider using `ast.literal_eval`"),
Reason::MarkSafe => format!("Use of `mark_safe` may expose cross-site scripting vulnerabilities"),
Reason::URLOpen => format!("Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected."),
Reason::NonCryptographicRandom => format!("Standard pseudo-random generators are not suitable for cryptographic purposes"),
Reason::UntrustedXML => format!("Using various XLM methods to parse untrusted XML data is known to be vulnerable to XML attacks; use `defusedxml` equivalents"),
Reason::UnverifiedSSL => format!("Python allows using an insecure context via the `_create_unverified_context` that reverts to the previous behavior that does not validate certificates or perform hostname checks"),
Reason::Telnet => format!("Telnet-related functions are being called. Telnet is considered insecure. Use SSH or some other encrypted protocol"),
Reason::FTPLib => format!("FTP-related functions are being called. FTP is considered insecure. Use SSH/SFTP/SCP or some other encrypted protocol"),
}
}
}
struct SuspiciousMembers<'a> {
members: &'a [&'a [&'a str]],
reason: Reason,
severity: Severity,
}
impl<'a> SuspiciousMembers<'a> {
pub const fn new(members: &'a [&'a [&'a str]], reason: Reason, severity: Severity) -> Self {
Self {
members,
reason,
severity,
}
}
}
struct SuspiciousModule<'a> {
name: &'a str,
reason: Reason,
severity: Severity,
}
impl<'a> SuspiciousModule<'a> {
pub const fn new(name: &'a str, reason: Reason, severity: Severity) -> Self {
Self {
name,
reason,
severity,
}
}
}
const SUSPICIOUS_MEMBERS: &[SuspiciousMembers] = &[
SuspiciousMembers::new(
&[
&["pickle", "loads"],
&["pickle", "load"],
&["pickle", "Unpickler"],
&["dill", "loads"],
&["dill", "load"],
&["dill", "Unpickler"],
&["shelve", "open"],
&["shelve", "DbfilenameShelf"],
&["jsonpickle", "decode"],
&["jsonpickle", "unpickler", "decode"],
&["pandas", "read_pickle"],
],
Reason::Pickle,
Severity::Medium,
),
SuspiciousMembers::new(
&[&["marshal", "loads"], &["marshal", "load"]],
Reason::Marshal,
Severity::Medium,
),
SuspiciousMembers::new(
&[
&["hashlib", "md5"],
&["hashlib", "sha1"],
&["Crypto", "Hash", "MD5", "new"],
&["Crypto", "Hash", "MD4", "new"],
&["Crypto", "Hash", "MD3", "new"],
&["Crypto", "Hash", "MD2", "new"],
&["Crypto", "Hash", "SHA", "new"],
&["Cryptodome", "Hash", "MD5", "new"],
&["Cryptodome", "Hash", "MD4", "new"],
&["Cryptodome", "Hash", "MD3", "new"],
&["Cryptodome", "Hash", "MD2", "new"],
&["Cryptodome", "Hash", "SHA", "new"],
&["cryptography", "hazmat", "primitives", "hashes", "MD5"],
&["cryptography", "hazmat", "primitives", "hashes", "SHA1"],
],
Reason::InsecureHash,
Severity::Medium,
),
SuspiciousMembers::new(
&[
&["Crypto", "Cipher", "ARC2", "new"],
&["Crypto", "Cipher", "ARC2", "new"],
&["Crypto", "Cipher", "Blowfish", "new"],
&["Crypto", "Cipher", "DES", "new"],
&["Crypto", "Cipher", "XOR", "new"],
&["Cryptodome", "Cipher", "ARC2", "new"],
&["Cryptodome", "Cipher", "ARC2", "new"],
&["Cryptodome", "Cipher", "Blowfish", "new"],
&["Cryptodome", "Cipher", "DES", "new"],
&["Cryptodome", "Cipher", "XOR", "new"],
&[
"cryptography",
"hazmat",
"primitives",
"ciphers",
"algorithms",
"ARC4",
],
&[
"cryptography",
"hazmat",
"primitives",
"ciphers",
"algorithms",
"Blowfish",
],
&[
"cryptography",
"hazmat",
"primitives",
"ciphers",
"algorithms",
"IDEA",
],
&[
"cryptography",
"hazmat",
"primitives",
"ciphers",
"modes",
"ECB",
],
],
Reason::InsecureCipher,
Severity::High,
),
SuspiciousMembers::new(&[&["tempfile", "mktemp"]], Reason::Mktemp, Severity::Medium),
SuspiciousMembers::new(&[&["eval"]], Reason::Eval, Severity::Medium),
SuspiciousMembers::new(
&[&["django", "utils", "safestring", "mark_safe"]],
Reason::MarkSafe,
Severity::Medium,
),
SuspiciousMembers::new(
&[
&["urllib", "urlopen"],
&["urllib", "request", "urlopen"],
&["urllib", "urlretrieve"],
&["urllib", "request", "urlretrieve"],
&["urllib", "URLopener"],
&["urllib", "request", "URLopener"],
&["urllib", "FancyURLopener"],
&["urllib", "request", "FancyURLopener"],
&["urllib2", "urlopen"],
&["urllib2", "Request"],
&["six", "moves", "urllib", "request", "urlopen"],
&["six", "moves", "urllib", "request", "urlretrieve"],
&["six", "moves", "urllib", "request", "URLopener"],
&["six", "moves", "urllib", "request", "FancyURLopener"],
],
Reason::URLOpen,
Severity::Medium,
),
SuspiciousMembers::new(
&[
&["random", "random"],
&["random", "randrange"],
&["random", "randint"],
&["random", "choice"],
&["random", "choices"],
&["random", "uniform"],
&["random", "triangular"],
],
Reason::NonCryptographicRandom,
Severity::Low,
),
SuspiciousMembers::new(
&[
&["xml", "etree", "cElementTree", "parse"],
&["xml", "etree", "cElementTree", "iterparse"],
&["xml", "etree", "cElementTree", "fromstring"],
&["xml", "etree", "cElementTree", "XMLParser"],
&["xml", "etree", "ElementTree", "parse"],
&["xml", "etree", "ElementTree", "iterparse"],
&["xml", "etree", "ElementTree", "fromstring"],
&["xml", "etree", "ElementTree", "XMLParser"],
&["xml", "sax", "expatreader", "create_parser"],
&["xml", "dom", "expatbuilder", "parse"],
&["xml", "dom", "expatbuilder", "parseString"],
&["xml", "sax", "parse"],
&["xml", "sax", "parseString"],
&["xml", "sax", "make_parser"],
&["xml", "dom", "minidom", "parse"],
&["xml", "dom", "minidom", "parseString"],
&["xml", "dom", "pulldom", "parse"],
&["xml", "dom", "pulldom", "parseString"],
&["lxml", "etree", "parse"],
&["lxml", "etree", "fromstring"],
&["lxml", "etree", "RestrictedElement"],
&["lxml", "etree", "GlobalParserTLS"],
&["lxml", "etree", "getDefaultParser"],
&["lxml", "etree", "check_docinfo"],
],
Reason::UntrustedXML,
Severity::High,
),
SuspiciousMembers::new(
&[&["ssl", "_create_unverified_context"]],
Reason::UnverifiedSSL,
Severity::Medium,
),
];
const SUSPICIOUS_MODULES: &[SuspiciousModule] = &[
SuspiciousModule::new("telnetlib", Reason::Telnet, Severity::High),
SuspiciousModule::new("ftplib", Reason::FTPLib, Severity::High),
];
/// S001
pub fn denied_function_call(checker: &mut Checker, expr: &Expr) {
let ExprKind::Call { func, .. } = &expr.node else {
return;
};
let Some(reason) = checker.ctx.resolve_call_path(func).and_then(|call_path| {
for module in SUSPICIOUS_MEMBERS {
if module.severity >= checker.settings.flake8_bandit.severity {
for member in module.members {
if call_path.as_slice() == *member {
return Some(module.reason);
}
}
}
}
for module in SUSPICIOUS_MODULES {
if module.severity >= checker.settings.flake8_bandit.severity {
if call_path.first() == Some(&module.name) {
return Some(module.reason);
}
}
}
None
}) else {
return;
};
let issue = DeniedFunctionCall { reason };
checker
.diagnostics
.push(Diagnostic::new(issue, Range::from(expr)));
}

View File

@ -1,217 +0,0 @@
use ruff_macros::{define_violation, derive_message_formats};
use rustpython_parser::ast::{Expr, ExprKind};
use smallvec::SmallVec;
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::registry::Diagnostic;
use crate::rules::flake8_bandit::helpers::Severity;
use crate::violation::Violation;
define_violation!(
pub struct DenylistCall {
pub message: String,
}
);
impl Violation for DenylistCall {
#[derive_message_formats]
fn message(&self) -> String {
format!("{}", self.message)
}
}
struct DLCall<'a> {
calls: &'a [&'a [&'a str]],
message: &'a str,
severity: Severity,
}
impl<'a> DLCall<'a> {
pub const fn new(calls: &'a [&'a [&'a str]], message: &'a str, severity: Severity) -> Self {
Self {
calls,
message,
severity,
}
}
}
// List comes from: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html
const DENYLIST_CALLS: &[DLCall] = &[
DLCall::new(
&[
&["pickle", "loads"],
&["pickle", "load"],
&["pickle", "Unpickler"],
&["dill", "loads"],
&["dill", "load"],
&["dill", "Unpickler"],
&["shelve", "open"],
&["shelve", "DbfilenameShelf"],
&["jsonpickle", "decode"],
&["jsonpickle", "unpickler", "decode"],
&["pandas", "read_pickle"],
],
"Pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue",
Severity::Medium
),
DLCall::new(
&[
&["marshal", "loads"],
&["marshal", "load"],
],
"Deserialization with the marshal module is possibly dangerous",
Severity::Medium
),
DLCall::new(
&[
&["hashlib", "md5"],
&["hashlib", "sha1"],
&["Crypto", "Hash", "MD5", "new"],
&["Crypto", "Hash", "MD4", "new"],
&["Crypto", "Hash", "MD3", "new"],
&["Crypto", "Hash", "MD2", "new"],
&["Crypto", "Hash", "SHA", "new"],
&["Cryptodome", "Hash", "MD5", "new"],
&["Cryptodome", "Hash", "MD4", "new"],
&["Cryptodome", "Hash", "MD3", "new"],
&["Cryptodome", "Hash", "MD2", "new"],
&["Cryptodome", "Hash", "SHA", "new"],
&["cryptography", "hazmat", "primitives", "hashes", "MD5"],
&["cryptography", "hazmat", "primitives", "hashes", "SHA1"],
],
"Use of insecure MD2, MD4, MD5, or SHA1 hash function",
Severity::Medium
),
DLCall::new(
&[
&["Crypto", "Cipher", "ARC2", "new"],
&["Crypto", "Cipher", "ARC2", "new"],
&["Crypto", "Cipher", "Blowfish", "new"],
&["Crypto", "Cipher", "DES", "new"],
&["Crypto", "Cipher", "XOR", "new"],
&["Cryptodome", "Cipher", "ARC2", "new"],
&["Cryptodome", "Cipher", "ARC2", "new"],
&["Cryptodome", "Cipher", "Blowfish", "new"],
&["Cryptodome", "Cipher", "DES", "new"],
&["Cryptodome", "Cipher", "XOR", "new"],
&["cryptography", "hazmat", "primitives", "ciphers", "algorithms", "ARC4"],
&["cryptography", "hazmat", "primitives", "ciphers", "algorithms", "Blowfish"],
&["cryptography", "hazmat", "primitives", "ciphers", "algorithms", "IDEA"],
&["cryptography", "hazmat", "primitives", "ciphers", "modes", "ECB"],
],
"Use of insecure cipher or cipher mode, replace with a known secure cipher such as AES",
Severity::High
),
DLCall::new(&[&["tempfile", "mktemp"]], "Use of insecure and deprecated function (mktemp)", Severity::Medium),
DLCall::new(&[&["eval"]], "Use of possibly insecure function - consider using safer ast.literal_eval", Severity::Medium),
DLCall::new(&[&["django", "utils", "safestring", "mark_safe"]], "Use of mark_safe() may expose cross-site scripting vulnerabilities and should be reviewed.", Severity::Medium),
DLCall::new(
&[
&["urllib", "urlopen"],
&["urllib", "request", "urlopen"],
&["urllib", "urlretrieve"],
&["urllib", "request", "urlretrieve"],
&["urllib", "URLopener"],
&["urllib", "request", "URLopener"],
&["urllib", "FancyURLopener"],
&["urllib", "request", "FancyURLopener"],
&["urllib2", "urlopen"],
&["urllib2", "Request"],
&["six", "moves", "urllib", "request", "urlopen"],
&["six", "moves", "urllib", "request", "urlretrieve"],
&["six", "moves", "urllib", "request", "URLopener"],
&["six", "moves", "urllib", "request", "FancyURLopener"],
],
"Audit url open for permitted schemes. Allowing use of file: or custom schemes is often unexpected",
Severity::Medium
),
DLCall::new(
&[
&["random", "random"],
&["random", "randrange"],
&["random", "randint"],
&["random", "choice"],
&["random", "choices"],
&["random", "uniform"],
&["random", "triangular"]
],
"Standard pseudo-random generators are not suitable for security/cryptographic purposes",
Severity::Low
),
DLCall::new(&[
&["xml", "etree", "cElementTree", "parse"],
&["xml", "etree", "cElementTree", "iterparse"],
&["xml", "etree", "cElementTree", "fromstring"],
&["xml", "etree", "cElementTree", "XMLParser"],
&["xml", "etree", "ElementTree", "parse"],
&["xml", "etree", "ElementTree", "iterparse"],
&["xml", "etree", "ElementTree", "fromstring"],
&["xml", "etree", "ElementTree", "XMLParser"],
&["xml", "sax", "expatreader", "create_parser"],
&["xml", "dom", "expatbuilder", "parse"],
&["xml", "dom", "expatbuilder", "parseString"],
&["xml", "sax", "parse"],
&["xml", "sax", "parseString"],
&["xml", "sax", "make_parser"],
&["xml", "dom", "minidom", "parse"],
&["xml", "dom", "minidom", "parseString"],
&["xml", "dom", "pulldom", "parse"],
&["xml", "dom", "pulldom", "parseString"],
&["lxml", "etree", "parse"],
&["lxml", "etree", "fromstring"],
&["lxml", "etree", "RestrictedElement"],
&["lxml", "etree", "GlobalParserTLS"],
&["lxml", "etree", "getDefaultParser"],
&["lxml", "etree", "check_docinfo"],
],
"Using various XLM methods to parse untrusted XML data is known to be vulnerable to XML attacks. Methods should be replaced with their defusedxml equivalents",
Severity::High
),
DLCall::new(&[&["ssl", "_create_unverified_context"]], "Python allows using an insecure context via the _create_unverified_context that reverts to the previous behavior that does not validate certificates or perform hostname checks", Severity::Medium)
];
fn comp_small_norm(small: &SmallVec<[&str; 8]>, norm: &&[&str]) -> bool {
if small.len() != norm.len() {
return false;
}
for (s, n) in small.iter().zip(norm.iter()) {
if s != n {
return false;
}
}
true
}
/// S001
pub fn denylist_calls(checker: &mut Checker, expr: &Expr) {
if let ExprKind::Call { func, .. } = &expr.node {
if let Some(message) = checker.ctx.resolve_call_path(func).and_then(|call_path| {
for bl_call in DENYLIST_CALLS {
if bl_call.severity >= checker.settings.flake8_bandit.severity {
for path in bl_call.calls {
if comp_small_norm(&call_path, path) {
return Some(bl_call.message);
}
}
}
}
if let Some(first_path) = call_path.first() {
// Both of these are high, so I am not adding conidtions for severity
if first_path == &"telnetlib" {
return Some("Telnet-related functions are being called. Telnet is considered insecure. Use SSH or some other encrypted protocol");
} else if first_path == &"ftplib" {
return Some("FTP-related functions are being called. FTP is considered insecure. Use SSH/SFTP/SCP or some other encrypted protocol");
}
}
None
}) {
let issue = DenylistCall {
message: message.to_string(),
};
checker
.diagnostics
.push(Diagnostic::new(issue, Range::from_located(expr)));
}
}
}

View File

@ -1,6 +1,6 @@
pub use assert_used::{assert_used, Assert};
pub use bad_file_permissions::{bad_file_permissions, BadFilePermissions};
pub use denylist_calls::{denylist_calls, DenylistCall};
pub use denied_function_call::{denied_function_call, DeniedFunctionCall};
pub use exec_used::{exec_used, ExecBuiltin};
pub use hardcoded_bind_all_interfaces::{
hardcoded_bind_all_interfaces, HardcodedBindAllInterfaces,
@ -31,7 +31,7 @@ pub use unsafe_yaml_load::{unsafe_yaml_load, UnsafeYAMLLoad};
mod assert_used;
mod bad_file_permissions;
mod denylist_calls;
mod denied_function_call;
mod exec_used;
mod hardcoded_bind_all_interfaces;
mod hardcoded_password_default;

View File

@ -5,7 +5,15 @@ use serde::{Deserialize, Serialize};
use ruff_macros::{CacheKey, ConfigurationOptions};
use crate::rules::flake8_bandit::helpers::Severity;
#[derive(
Debug, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema, Hash, CacheKey, PartialOrd,
)]
pub enum Severity {
#[default]
Low,
Medium,
High,
}
fn default_tmp_dirs() -> Vec<String> {
["/tmp", "/var/tmp", "/dev/shm"]
@ -46,8 +54,10 @@ pub struct Options {
/// exception types. By default, `try`-`except`-`pass` is only
/// disallowed for `Exception` and `BaseException`.
pub check_typed_exception: Option<bool>,
#[option(default = "", value_type = "str", example = "severity = low")]
/// The minimum severity to catch. Choose from `low`, `medium`, `high`,
#[option(default = "low", value_type = "str", example = "severity = \"high\"")]
/// The minimum severity to enforce for denied function calls.
///
/// Valid values are `low`, `medium`, and `high`.
pub severity: Option<Severity>,
}

View File

@ -3,8 +3,10 @@ source: crates/ruff/src/rules/flake8_bandit/mod.rs
expression: diagnostics
---
- kind:
DenylistCall:
message: "Pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue"
name: DeniedFunctionCall
body: "`pickle` and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue"
suggestion: ~
fixable: false
location:
row: 4
column: 0
@ -14,8 +16,10 @@ expression: diagnostics
fix: ~
parent: ~
- kind:
DenylistCall:
message: Telnet-related functions are being called. Telnet is considered insecure. Use SSH or some other encrypted protocol
name: DeniedFunctionCall
body: Telnet-related functions are being called. Telnet is considered insecure. Use SSH or some other encrypted protocol
suggestion: ~
fixable: false
location:
row: 6
column: 0

View File

@ -20,26 +20,6 @@ expression: diagnostics
body: "No builtin `eval()` allowed"
suggestion: ~
fixable: false
location:
row: 3
column: 0
end_location:
row: 3
column: 4
fix: ~
parent: ~
- kind:
NoEval: ~
location:
row: 9
column: 4
end_location:
row: 9
column: 8
fix: ~
parent: ~
- kind:
NoEval: ~
location:
row: 9
column: 4