Secure Code Review: Exploit-Ranked Findings

Critical

• auth.py:77-79

Admin check fails open. Any exception, including a missing X-User-Id header, logs a warning and returns True. Exploit: DELETE /admin/users/1 with no headers → req.headers["X-User-Id"] raises KeyError → is_admin returns True → the user is deleted. The same happens with X-User-Id: abc or a nonexistent id (fetchone() returns None, and None[0] raises). Fix: return False on any failure, and catch only the specific exceptions you expect.

• auth.py:70-75

Identity comes from client-controlled headers. X-Internal-Service: billing grants admin outright, and X-User-Id is trusted with no credential. /login returns only user_id and role, with no session or token, so nothing proves who the caller is. Exploit: DELETE /admin/users/5 with X-Internal-Service: billing, or X-User-Id: 1 where user 1 is an admin → admin access. Fix: issue a signed session or JWT at login and derive the user from it, with alg pinned and exp checked. Prove service callers with mTLS or a signed service token. Drop the header checks.

• auth.py:28-31

SQL injection in login. username is interpolated into the query. Exploit: {"username": "admin' --", "password": "x"} → the password clause is commented out and you get the admin row. {"username": "' OR 1=1 --", ...} returns the first user in the table. Fix: execute("SELECT id, role FROM users WHERE username = ? AND password_hash = ?", (username, hash_password(password))).

• auth.py:60

Password reset passes when the stored token is NULL. row[0] != token is None != None, which is False. Exploit: POST /password-reset/confirm {"email": "victim@x.com", "token": null, "new_password": "pwned"} → takes over any account with no pending reset. That is every account that has never requested a reset, and every account after a completed reset, since line 63 sets the token back to NULL. Fix: reject if the stored token is None or the submitted token isn't a non-empty string, then compare with hmac.compare_digest.

High

• auth.py:60-61

Reset expiry is never checked. reset_expires is selected and then ignored, and it is not cleared after use. Exploit: a token requested a year ago still works. This also widens the brute-force window below. Fix: require row[1] is not None and row[1] > time.time(), and clear reset_expires when you clear the token.

• auth.py:40

Reset token is predictable and brute-forceable. random.randint(100000, 999999) is not a CSPRNG and gives only 900k values, and there is no attempt limit on /confirm. Exploit: POST /password-reset/request for the victim, then loop /confirm over 100000–999999. Because expiry isn't enforced, there is no time limit either. Fix: secrets.token_urlsafe(32), store only its hash, and limit attempts per account and per IP.

• auth.py:20-21

Passwords hashed with unsalted MD5. Exploit: a leaked users.db is cracked with rainbow tables or GPU brute force in seconds, and identical passwords are visible as identical hashes. Fix: argon2id, bcrypt, or scrypt, via argon2-cffi, bcrypt, or hashlib.scrypt.

Medium

• auth.py:24,37,51

No rate limiting or lockout on /login, /password-reset/request, or /password-reset/confirm. Exploit: unlimited password guessing, and unlimited reset-email spam to a victim. Fix: per-account and per-IP limits, for example Flask-Limiter.

• auth.py:93

Reset token is printed to stdout. The [mail] stub writes the live token to logs. The token is also stored in plaintext at line 43. Exploit: anyone with log or database read access can take over accounts. Fix: remove the print once real mail is wired up, and store a hash of the token.

Low

• auth.py:56-66

Check-then-act race in /confirm. The SELECT and the UPDATE are separate, and the UPDATE doesn't re-check the token. Two concurrent requests with the same valid token both succeed. Fix: UPDATE ... WHERE email = ? AND reset_token = ? AND reset_expires > ?, then check rowcount == 1.

• auth.py:39-48 /password-reset/request sends mail to any submitted address, even one that isn't in the database. Exploit: use it to spam arbitrary inboxes. Fix: send only when the UPDATE matched a row, and keep the response identical either way.
• auth.py:26-27,39,52-54

A missing or non-JSON body raises KeyError or TypeError, which returns a 500. Fix: validate input and return a 400.

• auth.py:17 SQLite connections are never closed or reused. Use a context manager or g-scoped connection.
• Nothing in the file invalidates sessions on password reset, because no sessions exist. This is covered by the identity fix above.

I read the checklist items for identity, authorization, passwords, tokens, and sessions. I also checked the Python sinks. There is no shell, path, eval, or deserialization use, and the only other injection point is the login SQL. Error messages on login and reset don't reveal whether an account exists.

Fix first: is_admin at auth.py:70-79. Today, a bare DELETE /admin/users/<id> with no credentials deletes any account. Then fix the reset None == None bypass and the login SQL injection, since each gives full account takeover with one request.