Detect dangerous chmod with stat module constants in B103#1410
Open
jonasboos wants to merge 2 commits into
Open
Detect dangerous chmod with stat module constants in B103#1410jonasboos wants to merge 2 commits into
jonasboos wants to merge 2 commits into
Conversation
The B103 plugin only detected dangerous file permissions when chmod was called with integer literals (e.g., 0o777). When stat module constants like stat.S_IWOTH or expressions like stat.S_IWGRP | stat.S_IWOTH were used, the check was silently skipped because the AST node type was Attribute or BinOp, not int. Add _resolve_stat_expression() to recursively evaluate AST nodes that are stat constant references or bitwise-OR combinations of them. This enables detection of dangerous permissions expressed via the stat module. Resolves: PyCQA#1390
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 enhances the B103 “bad file permissions” plugin so it can detect dangerous chmod permission masks expressed using stat module constants (e.g., stat.S_IWOTH) and bitwise-OR expressions (e.g., stat.S_IWGRP | stat.S_IWOTH), closing a known false-negative gap.
Changes:
- Added
_resolve_stat_expression()to evaluate AST nodes representingstatconstants and|combinations into an integer mode. - Updated the example
os-chmod.pyto include astat.S_IWGRP | stat.S_IWOTHcase. - Updated functional test expectations to account for additional B103 findings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
bandit/plugins/general_bad_file_permissions.py |
Adds AST resolution logic for stat constants and bitwise-OR expressions to improve B103 detection coverage. |
examples/os-chmod.py |
Adds an example using stat constants combined via ` |
tests/functional/test_functional.py |
Updates expected severity/confidence counts for the os-chmod.py functional scan output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+67
to
+69
| # Only includes flags that are relevant to file permission checks. | ||
| _STAT_CONSTANTS = { | ||
| name: getattr(stat, name) for name in dir(stat) if name.startswith("S_I") |
| if isinstance(node, ast.BinOp) and isinstance(node.op, ast.BitOr): | ||
| left = _resolve_stat_expression(node.left) | ||
| right = _resolve_stat_expression(node.right) | ||
| if left is not None and right is not None: |
Comment on lines
+118
to
+122
| raw_args = context._context["call"].args | ||
| if len(raw_args) >= 2: | ||
| resolved = _resolve_stat_expression(raw_args[1]) | ||
| if resolved is not None: | ||
| mode = resolved |
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 B103 plugin only detects dangerous file permissions when is called with integer literals (e.g., ). When stat module constants like or bitwise-OR expressions like are used, the check is silently skipped because the AST node type is or , not .
As reported in #1390, code like this goes undetected:
Fix
Add
_resolve_stat_expression()that recursively evaluates AST nodes:ast.Attributenodes likestat.S_IWOTH→ resolved via lookup tableast.BinOp(BitOr) nodes likestat.S_IWGRP | stat.S_IWOTH→ resolved by evaluating both sidesThis enables B103 to flag dangerous permissions expressed via the stat module with the same severity/confidence levels as integer literal usage.
Testing
stat.S_IWGRP | stat.S_IWOTHto example fileResolves: #1390