Skip to content

Commit

Permalink
Merge branch 'main' into dhruv/custom-methods
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila committed Nov 24, 2023
2 parents 8b261bc + 017e829 commit 2155ecb
Show file tree
Hide file tree
Showing 21 changed files with 456 additions and 19 deletions.
65 changes: 65 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,65 @@
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 filter_data(filename):
tar = tarfile.open(filename)
tarfile.extractall(path=tempfile.mkdtemp(), filter="data")
tar.close()


def filter_fully_trusted(filename):
tar = tarfile.open(filename)
tarfile.extractall(path=tempfile.mkdtemp(), filter="fully_trusted")
tar.close()


def filter_tar(filename):
tar = tarfile.open(filename)
tarfile.extractall(path=tempfile.mkdtemp(), filter="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)
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,9 @@ class FakeEnum10(enum.Enum):
A = enum.auto()
B = enum.auto()
C = enum.auto()


class FakeEnum10(enum.Enum):
A = ...
B = ... # PIE796
C = ... # PIE796
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import enum


class FakeEnum1(enum.Enum):
A = ...
B = ...
C = ...
94 changes: 94 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E703.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": 1,
"id": "33faf7ad-a3fd-4ac4-a0c3-52e507ed49df",
"metadata": {},
"outputs": [],
"source": [
"x = 1"
]
},
{
"cell_type": "code",
"execution_count": 2,
"id": "481fb4bf-c1b9-47da-927f-3cfdfe4b49ec",
"metadata": {},
"outputs": [],
"source": [
"# Simple case\n",
"x;"
]
},
{
"cell_type": "code",
"execution_count": 3,
"id": "2f0c65a5-0a0e-4080-afce-5a8ed0d706df",
"metadata": {},
"outputs": [],
"source": [
"# Only skip the last expression\n",
"x; # E703\n",
"x;"
]
},
{
"cell_type": "code",
"execution_count": 4,
"id": "5a3fd75d-26d9-44f7-b013-1684aabfd0ae",
"metadata": {},
"outputs": [],
"source": [
"# Nested expressions isn't relevant\n",
"if True:\n",
" x;"
]
},
{
"cell_type": "code",
"execution_count": 5,
"id": "05eab5b9-e2ba-4954-8ef3-b035a79573fe",
"metadata": {},
"outputs": [],
"source": [
"# Semicolons with multiple expressions\n",
"x; x;"
]
},
{
"cell_type": "code",
"execution_count": 6,
"id": "9cbbddc5-83fc-4fdb-81ab-53a3912ae898",
"metadata": {},
"outputs": [],
"source": [
"# Comments, newlines and whitespace\n",
"x; # comment\n",
"\n",
"# another comment"
]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python (ruff-playground)",
"language": "python",
"name": "ruff-playground"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.3"
}
},
"nbformat": 4,
"nbformat_minor": 5
}
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 @@ -623,6 +623,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
16 changes: 13 additions & 3 deletions crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

use std::path::Path;

use ruff_notebook::CellOffsets;
use ruff_python_ast::PySourceType;
use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::Tok;

Expand All @@ -25,7 +27,8 @@ pub(crate) fn check_tokens(
locator: &Locator,
indexer: &Indexer,
settings: &LinterSettings,
is_stub: bool,
source_type: PySourceType,
cell_offsets: Option<&CellOffsets>,
) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = vec![];

Expand Down Expand Up @@ -108,7 +111,14 @@ pub(crate) fn check_tokens(
Rule::MultipleStatementsOnOneLineSemicolon,
Rule::UselessSemicolon,
]) {
pycodestyle::rules::compound_statements(&mut diagnostics, tokens, locator, indexer);
pycodestyle::rules::compound_statements(
&mut diagnostics,
tokens,
locator,
indexer,
source_type,
cell_offsets,
);
}

if settings.rules.enabled(Rule::AvoidableEscapedQuote) && settings.flake8_quotes.avoid_escape {
Expand Down Expand Up @@ -152,7 +162,7 @@ pub(crate) fn check_tokens(
pyupgrade::rules::extraneous_parentheses(&mut diagnostics, tokens, locator);
}

if is_stub && settings.rules.enabled(Rule::TypeCommentInStub) {
if source_type.is_stub() && settings.rules.enabled(Rule::TypeCommentInStub) {
flake8_pyi::rules::type_comment_in_stub(&mut diagnostics, locator, indexer);
}

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::Preview, 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
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ pub fn check_path(
locator,
indexer,
settings,
source_type.is_stub(),
source_type,
source_kind.as_ipy_notebook().map(Notebook::cell_offsets),
));
}

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,75 @@
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?
///
/// Extracting archives from untrusted sources without prior inspection is
/// a security risk, as maliciously crafted archives may contain files that
/// will be written outside of the target directory. For example, the archive
/// could include files with absolute paths (e.g., `/etc/passwd`), or relative
/// paths with parent directory references (e.g., `../etc/passwd`).
///
/// On Python 3.12 and later, use `filter='data'` to prevent the most dangerous
/// security issues (see: [PEP 706]). On earlier versions, set the `members`
/// argument to a trusted subset of the archive's members.
///
/// ## 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.extractall`](https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extractall)
/// - [Python Documentation: Extraction filters](https://docs.python.org/3/library/tarfile.html#tarfile-extraction-filter)
///
/// [PEP 706]: https://peps.python.org/pep-0706/#backporting-forward-compatibility
#[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 !call
.func
.as_attribute_expr()
.is_some_and(|attr| attr.attr.as_str() == "extractall")
{
return;
}

if call
.arguments
.find_keyword("filter")
.and_then(|keyword| keyword.value.as_string_literal_expr())
.is_some_and(|value| matches!(value.value.as_str(), "data" | "tar"))
{
return;
}

if !checker.semantic().seen(&["tarfile"]) {
return;
}

checker
.diagnostics
.push(Diagnostic::new(TarfileUnsafeMembers, call.func.range()));
}
Loading

0 comments on commit 2155ecb

Please sign in to comment.