Skip to content

feat: add B905 plugin for Flask send_file() with request-controlled path#1459

Open
Varshith-Kali wants to merge 2 commits into
PyCQA:mainfrom
Varshith-Kali:feat/flask-send-file-traversal
Open

feat: add B905 plugin for Flask send_file() with request-controlled path#1459
Varshith-Kali wants to merge 2 commits into
PyCQA:mainfrom
Varshith-Kali:feat/flask-send-file-traversal

Conversation

@Varshith-Kali

Copy link
Copy Markdown

Description

Adds a new bandit plugin (B905) that flags Flask's send_file() when the path argument is request-controlled — either directly (send_file(request.args.get("path"))) or via a simple local-variable assignment (path = request.args.get("path"); send_file(path)).

Closes #1403.

This pattern has been the root cause of multiple path-traversal / arbitrary-file-read CVEs:

Detection scope

Deliberately narrow, AST-level — no interprocedural dataflow:

  1. Direct case: send_file(...) where any argument expression contains a call to request.args.get / request.form.get / request.values.get. Matched by suffix so import aliases like flask_request.args.get also work.
  2. Local-variable case: send_file(name) where name is a bare Name whose most recent assignment in the enclosing function body is a request accessor call. Uses the _bandit_parent chain set by bandit's node visitor to find the enclosing FunctionDef, then walks its body for assignments to the target name.

Severity: MEDIUM (file read, not RCE — consistent with the issue's framing). Confidence: MEDIUM (consistent with B201 Flask debug). CWE: CWE-22 (Path Traversal).

What's NOT flagged (avoiding false positives)

  • send_file("/etc/hostname") — hardcoded path
  • send_file(os.path.join(base, rel)) — both components hardcoded
  • send_file() called without flask imported (the is_module_imported_like("flask") gate)
  • Local-variable case where the most recent assignment is NOT a request accessor (e.g. path = sanitize(user_input); send_file(path) — the sanitised assignment wins)

Files

File Change
bandit/plugins/flask_send_file.py New plugin (B905)
examples/flask_send_file.py 4 bad cases (direct args/form/values + local-var) + 2 okay cases
tests/functional/test_functional.py test_flask_send_file_with_request_path — expects 4 MEDIUM/MEDIUM issues
doc/source/plugins/b905_flask_send_file_with_request_path.rst Docs page (automodule)
setup.cfg Register flask_send_file_with_request_path entry point

Why B905

Verified B9xx range is unused by any existing plugin (B704 is the current max) and no in-flight PR claims B9xx (searched repo for "B9" in PR bodies). Happy to renumber if maintainers prefer a different convention (e.g. B205 to keep it in the Flask family next to B201).

Tests and Documentation

# 1. New functional test
$ python -m pytest tests/functional/test_functional.py -v -k "flask_send_file"
================= 1 passed, 79 deselected, 0.25s =================

# 2. Full functional suite (no regressions)
$ python -m pytest tests/functional/test_functional.py
================= 80 passed, 24 warnings in 0.44s =================

# 3. Unit tests (no regressions; 3 pre-existing collection errors from missing optional deps sarif_om/jsii — unrelated)
$ python -m pytest tests/unit/ --ignore=tests/unit/cli/test_baseline.py --ignore=tests/unit/formatters/test_html.py --ignore=tests/unit/formatters/test_sarif.py
================= 163 passed, 4 warnings in 0.52s =================

# 4. Run bandit on the example file — confirms 4 issues detected (4 bad cases)
$ bandit -r examples/flask_send_file.py
    Total issues (by severity): Medium: 4
    Total issues (by confidence): Medium: 4

# 5. Lint + format
$ flake8 bandit/plugins/flask_send_file.py
(clean)
$ black --check bandit/plugins/flask_send_file.py
All done. ✨ 🍰 ✨

AI assistance disclosure

Drafted with AI assistance (Claude), reviewed and tested by me before submission. I have read every line of the diff, run the full functional + unit test suites locally, and verified the plugin detects all 4 bad cases in the example file while not flagging the 2 safe cases. The detection approach (using the _bandit_parent chain to find the enclosing FunctionDef for the local-variable case, after discovering that context._context.get("function") is reset by pre_visit on every node) was arrived at by reading bandit's node_visitor.py source — the initial approach using the context dict didn't work because pre_visit clears it per-node.

Co-authored-by: Claude noreply@anthropic.com

Adds a new bandit plugin (B905) that flags Flask's send_file() when the
path argument is request-controlled — either directly
(send_file(request.args.get("path"))) or via a simple local-variable
assignment (path = request.args.get("path"); send_file(path)).

This pattern has been the root cause of multiple path-traversal /
arbitrary-file-read CVEs:
- CVE-2025-6166 (Agent-Zero)
- CVE-2022-31583
- CVE-2022-31549
- CVE-2022-31506

Detection scope (deliberately narrow, AST-level — no interprocedural
dataflow):
1. send_file(...) where any argument expression contains a call to
   request.args.get / request.form.get / request.values.get (matched
   by suffix so import aliases like flask_request.args.get also work).
2. send_file(name) where `name` is a bare Name whose most recent
   assignment in the enclosing function body is a request accessor
   call. Uses the _bandit_parent chain set by bandit's node visitor
   to find the enclosing FunctionDef, then walks its body for
   assignments to the target name.

Severity: MEDIUM (file read, not RCE). Confidence: MEDIUM (consistent
with B201 Flask debug). CWE-22 (Path Traversal).

Files:
- bandit/plugins/flask_send_file.py: new plugin
- examples/flask_send_file.py: 4 bad cases (direct args/form/values +
  local-var) + 2 okay cases (hardcoded path, os.path.join with
  hardcoded components)
- tests/functional/test_functional.py: test_flask_send_file_with_request_path
  expects 4 MEDIUM/MEDIUM issues
- doc/source/plugins/b905_flask_send_file_with_request_path.rst: docs page
- setup.cfg: register flask_send_file_with_request_path entry point

B905 chosen as the next free ID — verified B9xx range is unused by any
existing plugin (B704 is the current max) and no in-flight PR claims
B9xx (searched repo for "B9" in PR bodies). Happy to renumber if
maintainers prefer a different convention.

Tests: 1 new functional test passes. All 80 functional tests pass
(79 existing + 1 new). 163 unit tests pass (3 pre-existing collection
errors from missing optional deps sarif_om/jsii — unrelated to this
change). flake8 + black clean.

Closes PyCQA#1403

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Varshith Puli <pulivarshit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: Detecting Flask file-serving API misuse

1 participant