Skip to content

Commit

Permalink
[flake8-bandit] Implement tarfile-unsafe-members (S202)
Browse files Browse the repository at this point in the history
  • Loading branch information
ischaojie committed Nov 23, 2023
1 parent 5b726f7 commit 590908b
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 0 deletions.
47 changes: 47 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S202.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import sys
import tarfile
import tempfile


def unsafe_archive_handler(filename):
tar = tarfile.open(filename)
tar.extractall(path=tempfile.mkdtemp())
tar.close()


def managed_members_archive_handler(filename):
tar = tarfile.open(filename)
tar.extractall(path=tempfile.mkdtemp(), members=members_filter(tar))
tar.close()


def list_members_archive_handler(filename):
tar = tarfile.open(filename)
tar.extractall(path=tempfile.mkdtemp(), members=[])
tar.close()


def provided_members_archive_handler(filename):
tar = tarfile.open(filename)
tarfile.extractall(path=tempfile.mkdtemp(), members=tar)
tar.close()


def members_filter(tarfile):
result = []
for member in tarfile.getmembers():
if '../' in member.name:
print('Member name container directory traversal sequence')
continue
elif (member.issym() or member.islnk()) and ('../' in member.linkname):
print('Symlink to external resource')
continue
result.append(member)
return result


if __name__ == "__main__":
if len(sys.argv) > 1:
filename = sys.argv[1]
unsafe_archive_handler(filename)
managed_members_archive_handler(filename)
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 @@ -615,6 +615,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::DjangoRawSql) {
flake8_bandit::rules::django_raw_sql(checker, call);
}
if checker.enabled(Rule::TarfileUnsafeMembers) {
flake8_bandit::rules::tarfile_unsafe_members(checker, call);
}
if checker.enabled(Rule::UnnecessaryGeneratorList) {
flake8_comprehensions::rules::unnecessary_generator_list(
checker, expr, func, args, keywords,
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 @@ -595,6 +595,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "112") => (RuleGroup::Stable, rules::flake8_bandit::rules::TryExceptContinue),
(Flake8Bandit, "113") => (RuleGroup::Stable, rules::flake8_bandit::rules::RequestWithoutTimeout),
(Flake8Bandit, "201") => (RuleGroup::Preview, rules::flake8_bandit::rules::FlaskDebugTrue),
(Flake8Bandit, "202") => (RuleGroup::Stable, rules::flake8_bandit::rules::TarfileUnsafeMembers),
(Flake8Bandit, "301") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousPickleUsage),
(Flake8Bandit, "302") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousMarshalUsage),
(Flake8Bandit, "303") => (RuleGroup::Stable, rules::flake8_bandit::rules::SuspiciousInsecureHashUsage),
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 @@ -51,6 +51,7 @@ mod tests {
#[test_case(Rule::UnsafeYAMLLoad, Path::new("S506.py"))]
#[test_case(Rule::WeakCryptographicKey, Path::new("S505.py"))]
#[test_case(Rule::DjangoRawSql, Path::new("S611.py"))]
#[test_case(Rule::TarfileUnsafeMembers, Path::new("S202.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
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 tarfile_unsafe_members::*;
pub(crate) use try_except_continue::*;
pub(crate) use try_except_pass::*;
pub(crate) use unsafe_yaml_load::*;
Expand Down Expand Up @@ -49,6 +50,7 @@ mod snmp_insecure_version;
mod snmp_weak_cryptography;
mod ssh_no_host_key_verification;
mod suspicious_function_call;
mod tarfile_unsafe_members;
mod try_except_continue;
mod try_except_pass;
mod unsafe_yaml_load;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use crate::checkers::ast::Checker;
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast};
use ruff_text_size::Ranged;

/// ## What it does
/// Checks for uses of ``tarfile.extractall()`.
///
/// ## Why is this bad?
/// Use ``tarfile.extractall(members=function_name)`` and define a function
/// that will inspect each member. Discard files that contain a directory
/// traversal sequences such as ``../`` or ``\..`` along with all special filetypes
/// unless you explicitly need them.
///
/// ## Example
/// ```python
/// import tarfile
/// import tempfile
///
/// tar = tarfile.open(filename)
/// tar.extractall(path=tempfile.mkdtemp())
/// tar.close()
/// ```
///
/// ## References
/// - [Common Weakness Enumeration: CWE-22](https://cwe.mitre.org/data/definitions/22.html)
/// - [Python Documentation: tarfile](https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extractall)
#[violation]
pub struct TarfileUnsafeMembers;

impl Violation for TarfileUnsafeMembers {
#[derive_message_formats]
fn message(&self) -> String {
format!("Uses of `tarfile.extractall()`")
}
}

/// S202
pub(crate) fn tarfile_unsafe_members(checker: &mut Checker, call: &ast::ExprCall) {
if checker
.semantic()
.resolve_call_path(&call.func)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["tarfile", "extractall"]))
|| call
.func
.as_attribute_expr()
.is_some_and(|attr| attr.attr.as_str() == "extractall")
{
checker
.diagnostics
.push(Diagnostic::new(TarfileUnsafeMembers, call.func.range()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
assertion_line: 61
---
S202.py:8:5: S202 Uses of `tarfile.extractall()`
|
6 | def unsafe_archive_handler(filename):
7 | tar = tarfile.open(filename)
8 | tar.extractall(path=tempfile.mkdtemp())
| ^^^^^^^^^^^^^^ S202
9 | tar.close()
|

S202.py:14:5: S202 Uses of `tarfile.extractall()`
|
12 | def managed_members_archive_handler(filename):
13 | tar = tarfile.open(filename)
14 | tar.extractall(path=tempfile.mkdtemp(), members=members_filter(tar))
| ^^^^^^^^^^^^^^ S202
15 | tar.close()
|

S202.py:20:5: S202 Uses of `tarfile.extractall()`
|
18 | def list_members_archive_handler(filename):
19 | tar = tarfile.open(filename)
20 | tar.extractall(path=tempfile.mkdtemp(), members=[])
| ^^^^^^^^^^^^^^ S202
21 | tar.close()
|

S202.py:26:5: S202 Uses of `tarfile.extractall()`
|
24 | def provided_members_archive_handler(filename):
25 | tar = tarfile.open(filename)
26 | tarfile.extractall(path=tempfile.mkdtemp(), members=tar)
| ^^^^^^^^^^^^^^^^^^ S202
27 | tar.close()
|


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 590908b

Please sign in to comment.