Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added placeholder.py
Empty file.
48 changes: 48 additions & 0 deletions transfer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import sqlite3
from flask import request, jsonify
from functools import wraps


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


def register_routes(app):
@app.route("/api/transfer", methods=["POST"])
@token_required
def api_transfer(current_user):
data = request.get_json()
to_account = data.get("to_account")
amount = data.get("amount")

conn = sqlite3.connect("bank.db")
c = conn.cursor()

c.execute(f"SELECT balance FROM users WHERE id = {current_user['user_id']}")
balance = c.fetchone()[0]

if balance >= amount:
c.execute(
f"UPDATE users SET balance = balance - {amount} WHERE id = {current_user['user_id']}"
)
c.execute(
f"UPDATE users SET balance = balance + {amount} WHERE account_number='{to_account}'"
)
conn.commit()

c.execute(
f"SELECT username, balance FROM users WHERE account_number='{to_account}'"
)
recipient = c.fetchone()
conn.close()
return jsonify({"recipient": recipient[0], "new_balance": recipient[1]})

conn.close()
return jsonify({"error": "Insufficient funds"}), 400
Comment on lines +19 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL SQL Injection in fund transfer endpoint

The api_transfer function in transfer.py constructs SQL queries using f-strings with unsanitized user input from the request body (to_account and amount). This allows an attacker to inject arbitrary SQL commands, potentially leading to unauthorized balance modifications or data exfiltration. Specifically, the to_account field is concatenated directly into the SQL query strings on lines 35 and 40.

Steps to Reproduce
An attacker can send a POST request to `/api/transfer` with a JSON payload where `to_account` contains a SQL injection payload, such as `{"to_account": "' OR 1=1 --", "amount": 100}`. This will alter the query executed on line 35 to `UPDATE users SET balance = balance + 100 WHERE account_number='' OR 1=1 --'`.
Fix with AI

Open in Cursor Open in Claude

A security vulnerability was found by Hacktron.

File: transfer.py
Lines: 19-47
Severity: critical

Vulnerability: SQL Injection in fund transfer endpoint

Description:
The `api_transfer` function in `transfer.py` constructs SQL queries using f-strings with unsanitized user input from the request body (`to_account` and `amount`). This allows an attacker to inject arbitrary SQL commands, potentially leading to unauthorized balance modifications or data exfiltration. Specifically, the `to_account` field is concatenated directly into the SQL query strings on lines 35 and 40.

Proof of Concept:
An attacker can send a POST request to `/api/transfer` with a JSON payload where `to_account` contains a SQL injection payload, such as `{"to_account": "' OR 1=1 --", "amount": 100}`. This will alter the query executed on line 35 to `UPDATE users SET balance = balance + 100 WHERE account_number='' OR 1=1 --'`.

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.

View finding in Hacktron

Comment on lines +19 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH Race condition in fund transfer logic

The api_transfer function performs a balance check followed by two separate update operations without any transaction-level locking or atomic operations, despite using SQLite. An attacker could potentially trigger multiple concurrent requests to transfer more money than is available in their account, leading to a race condition where the balance becomes negative. The conn.commit() is called only after both updates, but the check and the updates are not atomic in the context of concurrent requests.

Steps to Reproduce
Send multiple concurrent POST requests to `/api/transfer` with the same `amount` that is close to the total balance. If the requests are processed in parallel, the `SELECT` query on line 27 might return the same initial balance for all requests before any of the `UPDATE` queries on line 32 have committed, allowing the user to bypass the `if balance >= amount` check multiple times.
Fix with AI

Open in Cursor Open in Claude

A security vulnerability was found by Hacktron.

File: transfer.py
Lines: 19-47
Severity: high

Vulnerability: Race condition in fund transfer logic

Description:
The `api_transfer` function performs a balance check followed by two separate update operations without any transaction-level locking or atomic operations, despite using SQLite. An attacker could potentially trigger multiple concurrent requests to transfer more money than is available in their account, leading to a race condition where the balance becomes negative. The `conn.commit()` is called only after both updates, but the check and the updates are not atomic in the context of concurrent requests.

Proof of Concept:
Send multiple concurrent POST requests to `/api/transfer` with the same `amount` that is close to the total balance. If the requests are processed in parallel, the `SELECT` query on line 27 might return the same initial balance for all requests before any of the `UPDATE` queries on line 32 have committed, allowing the user to bypass the `if balance >= amount` check multiple times.

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.

View finding in Hacktron