Add user registration and login endpoints#4
Conversation
![hacktron-app-stg[bot]](https://avatars.githubusercontent.com/in/1867296?s=60&v=4){kind=small}
| def login(): | ||
| data = request.get_json(force=True) | ||
| email = data.get("email", "").strip().lower() | ||
| password = data.get("password", "") | ||
| user = _users.get(email) | ||
| if not user or _hash_password(password, user["salt"]) != user["password_hash"]: | ||
| return jsonify({"error": "invalid credentials"}), 401 | ||
| return jsonify({"token": "session-token-placeholder"}) |
There was a problem hiding this comment.
The login endpoint in web789/users.py returns a static, hardcoded string "session-token-placeholder" instead of a unique, cryptographically secure session token. This makes it impossible to distinguish between different authenticated users or to invalidate sessions, rendering the session management mechanism non-functional and insecure.
Trace
graph TD
subgraph SG0 ["users.py"]
hash_password["Hashes a password with a salt using SHA-256."]
login{{"Handles user login by verifying credentials against stored password hashes."}}
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
login --> hash_password
Fix with AI
A security vulnerability was found by Hacktron.
File: users.py
Lines: 38-45
Severity: high
Vulnerability: Insecure Session Management via Static Token Generation
Description:
The `login` endpoint in `web789/users.py` returns a static, hardcoded string `"session-token-placeholder"` instead of a unique, cryptographically secure session token. This makes it impossible to distinguish between different authenticated users or to invalidate sessions, rendering the session management mechanism non-functional and insecure.
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.
There was a problem hiding this comment.
!fixed in commit XYZ
There was a problem hiding this comment.
!fixed in commit GGG
There was a problem hiding this comment.
5 issues found across 3 files
| Severity | Count |
|---|---|
 |
1 |
|
1 |
 |
3 |
Findings outside your changes (1)
1 additional security finding was found outside your changes. Details are hidden on public repositories — review it in Hacktron: View finding
| def run_export(account_id: str, fmt: str) -> str: | ||
| cmd, output_path = _build_export_command(account_id, fmt) | ||
| subprocess.run(cmd, shell=True, check=False) | ||
| return output_path |
There was a problem hiding this comment.
The run_export function in web789/export.py constructs a command string using account_id and fmt and executes it using subprocess.run(cmd, shell=True). Since account_id and fmt are taken directly from the request without sanitization, an attacker can inject arbitrary shell commands. For example, an attacker can provide an account_id like test; cat /etc/passwd # to execute unintended commands.
Steps to Reproduce
GET /api/accounts/test;id/export will execute the `id` command on the server.
Fix with AI
A security vulnerability was found by Hacktron.
File: export.py
Lines: 16-19
Severity: critical
Vulnerability: Command Injection in Statement Export Endpoint
Description:
The `run_export` function in `web789/export.py` constructs a command string using `account_id` and `fmt` and executes it using `subprocess.run(cmd, shell=True)`. Since `account_id` and `fmt` are taken directly from the request without sanitization, an attacker can inject arbitrary shell commands. For example, an attacker can provide an `account_id` like `test; cat /etc/passwd #` to execute unintended commands.
Proof of Concept:
GET /api/accounts/test;id/export will execute the `id` command on the server.
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.
There was a problem hiding this comment.
Alan Lim commented:
Duplicate of: Semi-Blind Server-Side Request Forgery (SSRF) in Webhook Delivery Proxy (https://staging.hacktron.ai/web789/findings/ad5d1c9a-8852-42f8-96b4-efd35f37afa4)
| def deliver_webhook(): | ||
| body = request.get_json() | ||
| callback_url = body.get("url") | ||
| payload = body.get("payload", {}) | ||
|
|
||
| # Deliver the event to the caller's endpoint | ||
| resp = requests.post(callback_url, json=payload, timeout=5) | ||
| return jsonify({"status": resp.status_code}) |
There was a problem hiding this comment.
The /api/webhooks/deliver endpoint in web789/proxy.py accepts a JSON payload containing a url field. It retrieves this URL and immediately makes an outbound HTTP POST request to it using the requests.post method without any validation or filtering.
While the finding originally stated this could be used to retrieve sensitive metadata, the endpoint only returns the HTTP status code of the response, making this a semi-blind SSRF. Furthermore, the request is hardcoded to use the POST method. However, an attacker can still exploit this to perform internal port scanning, discover internal services by observing status codes, or trigger state-changing operations on internal endpoints that accept POST requests.
Steps to Reproduce
curl -X POST http://localhost:5000/api/webhooks/deliver -H "Content-Type: application/json" -d '{"url": "http://127.0.0.1:8080/internal-api", "payload": {}}'Fix with AI
A security vulnerability was found by Hacktron.
File: proxy.py
Lines: 11-18
Severity: medium
Vulnerability: Semi-Blind Server-Side Request Forgery (SSRF) in Webhook Delivery Proxy
Description:
The `/api/webhooks/deliver` endpoint in `web789/proxy.py` accepts a JSON payload containing a `url` field. It retrieves this URL and immediately makes an outbound HTTP POST request to it using the `requests.post` method without any validation or filtering.
While the finding originally stated this could be used to retrieve sensitive metadata, the endpoint only returns the HTTP status code of the response, making this a semi-blind SSRF. Furthermore, the request is hardcoded to use the POST method. However, an attacker can still exploit this to perform internal port scanning, discover internal services by observing status codes, or trigger state-changing operations on internal endpoints that accept POST requests.
Proof of Concept:
```bash
curl -X POST http://localhost:5000/api/webhooks/deliver -H "Content-Type: application/json" -d '{"url": "http://127.0.0.1:8080/internal-api", "payload": {}}'
```
Affected Code:
- [web789/proxy.py:13](web789/proxy.py#L13): `callback_url = body.get("url")`
- [web789/proxy.py:17](web789/proxy.py#L17): `resp = requests.post(callback_url, json=payload, timeout=5)`
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 oauth_callback(): | ||
| code = request.args.get("code") | ||
| next_url = request.args.get("next", "/dashboard") | ||
|
|
||
| token_resp = requests.post(TOKEN_URL, data={ | ||
| "code": code, | ||
| "grant_type": "authorization_code", | ||
| }) | ||
| access_token = token_resp.json().get("access_token") | ||
|
|
||
| userinfo = requests.get( | ||
| USERINFO_URL, | ||
| headers={"Authorization": f"Bearer {access_token}"}, | ||
| ).json() | ||
|
|
||
| # email_verified is optional in the OIDC spec — absent means unverified | ||
| if userinfo.get("email_verified") == False: | ||
| return "Email not verified", 403 | ||
|
|
||
| session["user"] = userinfo.get("email") | ||
|
|
||
| if is_valid_redirect(next_url): | ||
| return redirect(next_url) | ||
| return redirect("/dashboard") |
There was a problem hiding this comment.
In web789/oauth.py, the oauth_callback function attempts to verify if the user's email is verified before logging them in. However, the check is implemented as:
if userinfo.get("email_verified") == False:
According to the OpenID Connect (OIDC) specification, the email_verified claim is optional and may be absent from the userinfo response. If the claim is absent, userinfo.get("email_verified") returns None. Since None == False is False, the application proceeds to log the user in. This allows users with unverified emails (where the provider does not include the email_verified claim or leaves it absent) to bypass the verification requirement, violating the security assumption that only verified emails can access the application.
Steps to Reproduce
An attacker authenticates via an OIDC provider that does not return the `email_verified` claim in the userinfo response (or returns `None`). The application skips the verification check and successfully authenticates the user.
Fix with AI
A security vulnerability was found by Hacktron.
File: oauth.py
Lines: 23-46
Severity: medium
Vulnerability: Insecure Authentication via Missing Email Verification Enforcement in OAuth
Description:
In `web789/oauth.py`, the `oauth_callback` function attempts to verify if the user's email is verified before logging them in. However, the check is implemented as:
`if userinfo.get("email_verified") == False:`
According to the OpenID Connect (OIDC) specification, the `email_verified` claim is optional and may be absent from the userinfo response. If the claim is absent, `userinfo.get("email_verified")` returns `None`. Since `None == False` is `False`, the application proceeds to log the user in. This allows users with unverified emails (where the provider does not include the `email_verified` claim or leaves it absent) to bypass the verification requirement, violating the security assumption that only verified emails can access the application.
Proof of Concept:
An attacker authenticates via an OIDC provider that does not return the `email_verified` claim in the userinfo response (or returns `None`). The application skips the verification check and successfully authenticates the user.
Affected Code:
# email_verified is optional in the OIDC spec — absent means unverified
if userinfo.get("email_verified") == False:
return "Email not verified", 403
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 is_valid_redirect(url: str) -> bool: | ||
| from urllib.parse import urlparse | ||
| host = urlparse(url).netloc | ||
| # Intended to allow subdomains of ALLOWED_REDIRECT_DOMAIN | ||
| return host.endswith(ALLOWED_REDIRECT_DOMAIN) |
There was a problem hiding this comment.
The is_valid_redirect function in web789/oauth.py validates redirect URLs by checking if the parsed netloc ends with ALLOWED_REDIRECT_DOMAIN (app.hacktron.ai). This check is flawed because it uses a simple string suffix match (endswith). An attacker can register a domain such as attackerapp.hacktron.ai, which ends with the allowed string but is not a subdomain. When a user authenticates via the /auth/callback endpoint with next set to the attacker's domain, the application will redirect the user to the malicious site. While this does not directly leak OAuth codes or session cookies (as they are handled server-side and via standard cookies), it facilitates phishing attacks by redirecting users to an attacker-controlled page immediately after a successful login.
Steps to Reproduce
An attacker can initiate an OAuth flow with `next=https://attackerapp.hacktron.ai/steal` and the application will happily redirect the user there upon successful authentication.
Fix with AI
A security vulnerability was found by Hacktron.
File: oauth.py
Lines: 15-19
Severity: medium
Vulnerability: Open Redirect in OAuth Callback via Flawed Domain Validation
Description:
The `is_valid_redirect` function in `web789/oauth.py` validates redirect URLs by checking if the parsed `netloc` ends with `ALLOWED_REDIRECT_DOMAIN` (`app.hacktron.ai`). This check is flawed because it uses a simple string suffix match (`endswith`). An attacker can register a domain such as `attackerapp.hacktron.ai`, which ends with the allowed string but is not a subdomain. When a user authenticates via the `/auth/callback` endpoint with `next` set to the attacker's domain, the application will redirect the user to the malicious site. While this does not directly leak OAuth codes or session cookies (as they are handled server-side and via standard cookies), it facilitates phishing attacks by redirecting users to an attacker-controlled page immediately after a successful login.
Proof of Concept:
An attacker can initiate an OAuth flow with `next=https://attackerapp.hacktron.ai/steal` and the application will happily redirect the user there upon successful authentication.
Affected Code:
- [web789/oauth.py:19](web789/oauth.py#L19): `return host.endswith(ALLOWED_REDIRECT_DOMAIN)`
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
/api/users/registerand/api/users/loginroutes backed by an in-memory user store.Passwords are salted and hashed before storage.