Detect unsafe tarfile.extract() in B202 plugin#1409
Open
jonasboos wants to merge 2 commits into
Open
Conversation
The B202 plugin only detected tarfile.extractall() calls but missed equally unsafe tarfile.extract() calls, which have the same path traversal risk (CWE-22) when members are not validated. Add detection for tarfile.extract() with HIGH severity and confidence. The filter='data' keyword is respected as a safe pattern, same as for extractall. Add example code and update functional tests. Resolves: PyCQA#1392
jonasboos
requested review from
ericwb,
lukehinds and
sigmavirus24
as code owners
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This pull request extends Bandit’s B202 plugin to also flag unsafe tarfile.extract() usage (path traversal risk) in addition to existing tarfile.extractall() detection, and updates examples + functional test expectations accordingly.
Changes:
- Add B202 detection for
tarfile.extract()calls (unlessfilter="data"is present). - Update the
examples/tarfile_extractall.pyexample file to includeextract()scenarios. - Update functional test expected severity/confidence counts for the example scan.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
bandit/plugins/tarfile_unsafe_members.py |
Adds extract() detection and refactors issue construction helpers/messages. |
examples/tarfile_extractall.py |
Adds new example functions covering tar.extract() with/without filter="data". |
tests/functional/test_functional.py |
Updates expected finding counts for the tarfile example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+124
to
+138
| if "extractall" in context.call_function_name: | ||
| if "filter" in context.call_keywords and is_filter_data(context): | ||
| return None | ||
| if "members" in context.call_keywords: | ||
| members = get_members_value(context) | ||
| if "Function" in members: | ||
| return exec_issue(bandit.LOW, members) | ||
| return exec_issue_extractall(bandit.LOW, members) | ||
| else: | ||
| return exec_issue(bandit.MEDIUM, members) | ||
| return exec_issue(bandit.HIGH) | ||
| return exec_issue_extractall(bandit.MEDIUM, members) | ||
| return exec_issue_extractall(bandit.HIGH) | ||
|
|
||
| if "extract" in context.call_function_name: | ||
| if "filter" in context.call_keywords and is_filter_data(context): | ||
| return None | ||
| return exec_issue_extract() |
Comment on lines
5
to
13
| ================================= | ||
| B202: Test for tarfile.extractall | ||
| ================================= | ||
|
|
||
| This plugin will look for usage of ``tarfile.extractall()`` | ||
| This plugin will look for usage of ``tarfile.extractall()`` and | ||
| ``tarfile.extract()`` | ||
|
|
||
| Severity are set as follows: | ||
|
|
Comment on lines
+89
to
+97
| def exec_issue_extract(): | ||
| return bandit.Issue( | ||
| severity=bandit.HIGH, | ||
| confidence=bandit.HIGH, | ||
| cwe=issue.Cwe.PATH_TRAVERSAL, | ||
| text="tarfile.extract used without member validation. " | ||
| "Untarred members should be inspected for directory traversal " | ||
| "sequences such as '../' and dangerous file types.", | ||
| ) |
Author
|
@copilot apply changes based on the comments in this thread |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The B202 plugin only detects calls but misses equally unsafe calls. Both methods have the same path traversal risk (CWE-22) when members are not validated.
As reported in #1392, code like this goes undetected:
Fix
Add detection for with the same severity model:
Also renames the internal function to for clarity and adds a separate function with an appropriate message.
Testing
Resolves: #1392