Compare commits

...

6 Commits

Author SHA1 Message Date
colin99d
6c2d430430 Updated output format 2023-03-21 13:31:49 -04:00
colin99d
b0242ccfa1 Updated tests 2023-03-21 12:22:38 -04:00
colin99d
8ffc77d0ee Got tests working 2023-03-21 12:22:38 -04:00
colin99d
adc677bc9c Got tests working 2023-03-21 12:22:27 -04:00
colin99d
213e83aff2 Made small changes 2023-03-21 12:22:00 -04:00
colin99d
4b91826a0b Everything ready 2023-03-21 12:21:41 -04:00
10 changed files with 384 additions and 0 deletions

View File

@@ -0,0 +1,6 @@
import pickle
from telnetlib import Telnet
pickle.loads()
Telnet("localhost", 23)

View File

@@ -2444,6 +2444,11 @@ where
flake8_print::rules::print_call(self, func, keywords);
}
// flake8-bandit
if self.settings.rules.enabled(Rule::DeniedFunctionCall) {
flake8_bandit::rules::denied_function_call(self, expr);
}
// flake8-bugbear
if self.settings.rules.enabled(Rule::UnreliableCallableCheck) {
flake8_bugbear::rules::unreliable_callable_check(self, expr, func, args);

View File

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

View File

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

View File

@@ -12,9 +12,11 @@ mod tests {
use test_case::test_case;
use crate::registry::Rule;
use crate::rules::flake8_bandit::settings::Severity;
use crate::settings::Settings;
use crate::test::test_path;
#[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")]
@@ -57,6 +59,7 @@ mod tests {
"/foo".to_string(),
],
check_typed_exception: false,
severity: Severity::Low,
},
..Settings::for_rule(Rule::HardcodedTempFile)
},

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,5 +1,6 @@
pub use assert_used::{assert_used, Assert};
pub use bad_file_permissions::{bad_file_permissions, BadFilePermissions};
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,
@@ -30,6 +31,7 @@ pub use unsafe_yaml_load::{unsafe_yaml_load, UnsafeYAMLLoad};
mod assert_used;
mod bad_file_permissions;
mod denied_function_call;
mod exec_used;
mod hardcoded_bind_all_interfaces;
mod hardcoded_password_default;

View File

@@ -5,6 +5,16 @@ use serde::{Deserialize, Serialize};
use ruff_macros::{CacheKey, ConfigurationOptions};
#[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"]
.map(std::string::ToString::to_string)
@@ -44,12 +54,18 @@ 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 = "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>,
}
#[derive(Debug, CacheKey)]
pub struct Settings {
pub hardcoded_tmp_directory: Vec<String>,
pub check_typed_exception: bool,
pub severity: Severity,
}
impl From<Options> for Settings {
@@ -67,6 +83,7 @@ impl From<Options> for Settings {
)
.collect(),
check_typed_exception: options.check_typed_exception.unwrap_or(false),
severity: options.severity.unwrap_or_default(),
}
}
}
@@ -77,6 +94,7 @@ impl From<Settings> for Options {
hardcoded_tmp_directory: Some(settings.hardcoded_tmp_directory),
hardcoded_tmp_directory_extend: None,
check_typed_exception: Some(settings.check_typed_exception),
severity: Some(settings.severity),
}
}
}
@@ -86,6 +104,7 @@ impl Default for Settings {
Self {
hardcoded_tmp_directory: default_tmp_dirs(),
check_typed_exception: false,
severity: Severity::default(),
}
}
}

View File

@@ -0,0 +1,31 @@
---
source: crates/ruff/src/rules/flake8_bandit/mod.rs
expression: diagnostics
---
- kind:
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
end_location:
row: 4
column: 14
fix: ~
parent: ~
- kind:
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
end_location:
row: 6
column: 23
fix: ~
parent: ~

22
ruff.schema.json generated
View File

@@ -630,6 +630,17 @@
"items": {
"type": "string"
}
},
"severity": {
"description": "The minimum severity to catch. Choose from `low`, `medium`, `high`,",
"anyOf": [
{
"$ref": "#/definitions/Severity"
},
{
"type": "null"
}
]
}
},
"additionalProperties": false
@@ -2031,6 +2042,9 @@
"RUF10",
"RUF100",
"S",
"S0",
"S00",
"S001",
"S1",
"S10",
"S101",
@@ -2229,6 +2243,14 @@
"azure"
]
},
"Severity": {
"type": "string",
"enum": [
"Low",
"Medium",
"High"
]
},
"Strictness": {
"oneOf": [
{