Conversation
PUT /users/v1/{username}/password previously used the URL path
username directly to look up and overwrite a user's password,
allowing any authenticated user to hijack any other account by
supplying a different username in the URL while presenting their
own valid JWT.
Now the handler compares the URL username against the authenticated
caller's own subject (resp['sub']) taken from their validated token,
and returns 403 if they don't match, before any password update is
performed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
User.get_user() built a raw SQL string via f-string interpolation of
the username path param and executed it directly, allowing classic
UNION/boolean-based SQL injection (API8:2019 Injection).
Switch to a parameterized query using SQLAlchemy's text() bound
parameter (:username) so the value is always passed as data, never
concatenated into the SQL statement.
Verified locally: UNION/boolean injection payloads against
GET /users/v1/{username} (e.g. "name1' OR '1'='1", UNION SELECT
dumping the admin row, and a bare trailing quote) all now resolve as
'User not found' with no SQL error, while legitimate lookups for
name1, name2, and admin still return the correct user JSON.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
POST /users/v1/login previously returned two distinguishable error messages in the vulnerable branch: 'Username does not exist' when the username was not found, and 'Password is not correct for the given username' when it was found but the password did not match. This let an attacker enumerate valid usernames and use the login endpoint as a password oracle. Now both failure cases return the same generic 'Username or Password Incorrect!' message, matching the pattern used elsewhere in the code for the non-enumerable case, so the response no longer leaks whether the supplied username exists.
SECRET_KEY was a fixed literal ('random') committed to source, so anyone
could forge valid auth tokens offline for any username (including admin)
without ever logging in.
Now the key is sourced from the SECRET_KEY environment variable when the
operator provides one, and otherwise falls back to a securely-generated
random key per process (secrets.token_hex(32)) so there is never a
shared, guessable default baked into the repo.
Verified locally: a token forged with the old hardcoded key 'random' is
now rejected with 401 Invalid token, while tokens issued by the running
app's own /login endpoint continue to authenticate normally.
PUT /users/v1/{username}/email validated the new email against a
catastrophic-backtracking pattern: ^([0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*@{1}...)$
The nested (X*Y)* construction over overlapping character classes lets a
crafted string with no '@' drive exponential backtracking, hanging the
worker thread (ReDoS / API4:2019 Lack of Resources & Rate Limiting).
Replace it with an equivalent-intent email pattern where every repetition
is bounded (no unbounded quantifier is nested inside another), so
backtracking work stays linear in input length, plus a hard 254-char
length cap on the input as defense in depth. Verified locally that a
crafted payload that hung the original regex for 60+ seconds at n=30 now
gets rejected in milliseconds even at 250+ chars, while legitimate email
updates (e.g. name1@example.com) still succeed with HTTP 204.
The book-lookup endpoint returned any book's secret_content to any authenticated caller, keyed only on book_title with no ownership check against the requester. Enforce object-level authorization unconditionally: resolve the requesting user from the validated token's subject, look up the candidate book, and require the book's user_id to match the requester's id before returning its secret. Non-owned or non-existent titles both return a generic 404 so the endpoint doesn't leak which titles exist. Verified locally: logging in as name1 and requesting name2's and admin's books now returns 404 Book not found, while name1's own book still returns 200 with its secret.
- Require a valid admin bearer token to access the debug endpoint (previously unauthenticated, dumping every user's plaintext password and admin flag to anyone). - Enforce auth at both the OpenAPI layer (bearerAuth security requirement, matching other protected routes) and inside debug() itself (403 for authenticated non-admins). - Stop serializing the plaintext password field at all in User.json_debug(), even for admins - full account dumps should never echo raw credentials back over the wire. Fixes challenge 5 (API3:2019 Excessive Data Exposure). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
POST /users/v1/register previously took an 'admin' boolean straight from the client-supplied JSON body and used it to set the new user's privilege level, letting any anonymous caller register themselves as an administrator. New accounts are now always created as non-admin regardless of any extra 'admin' (or other) field present in the request body.
🏆 VAmPI — CTF Patch Score8 / 9 challenges patched
Commit: 🎉 Your result is on the leaderboard — see where you rank! 🏆 |
This was referenced Aug 9, 2026
POST /users/v1/login had no throttling, lockout, or backoff, allowing unlimited credential brute-forcing. Adds an in-memory lockout keyed by client IP + attempted username: after 5 failed attempts within a 60s window, further attempts (including with the correct password) get a 429 with Retry-After until the window expires. Successful login clears the counter for that key; unrelated accounts/IPs are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…challenge-6 generic error)
beanbeah
force-pushed
the
ctf/consolidated
branch
from
August 9, 2026 14:01
adce03d to
08561a6
Compare
…o code change, tree identical to 08561a6)
…t per-account
The account-scoped (IP, username) lockout added for Challenge 8 left the login
endpoint effectively unthrottled against two adversarial patterns that don't
reuse the same username:
- credential spraying / brute-forcing while varying the attempted username
on every request never accumulates enough failures against any single
(IP, username) key to trip the lockout (verified: 39 spray attempts with
zero throttling before this fix)
- flooding the endpoint with already-correct credentials is never counted
at all, since only failed attempts were tracked (verified: 79 successful
logins with zero throttling before this fix)
Adds two additional IP-scoped sliding-window counters alongside the existing
per-account one: a looser failed-attempt budget across all usernames from one
IP (catches spraying), and a total-attempt budget counting every login call
regardless of outcome (catches flooding with valid creds). Any of the three
tripping returns 429 with Retry-After, matching the existing response shape.
Verified all three brute-force paths are now blocked, legitimate low-volume
login traffic for all seeded accounts is unaffected, and the other 8 fixed
challenges are unaffected by this change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Author
|
Closing as part of a full stand-down of this CTF push. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidates all 9 validated VAmPI CTF challenge fixes from individual PRs (#65-#73) into a single PR against
dc34-ctf.Closes/supersedes the following individually-verified fixes:
SQL Injection in username lookup (API8:2019 Injection) —
models/user_model.pyUser.get_user(): parameterized the raw SQL with SQLAlchemytext()+ bound:usernameparam instead of f-string interpolation. (was PR Fix SQL Injection in GET /users/v1/{username} (API8:2019 Injection) #66)Unauthorized Password Change / BOLA (API1:2019) —
api_views/users.pyupdate_password(): now returns 403 if the URLusernamedoes not match the authenticated caller's own username (resp['sub']). (was PR Fix BOLA: PUT /users/v1/{username}/password can no longer change another user's password #65)BOLA on Books / secret leak (API1:2019) —
api_views/books.pyget_by_title(): now requires the requested book'suser_idto match the authenticated requester's id, else a generic 404, regardless of the legacyvulntoggle. (was PR Fix BOLA: GET /books/v1/{book_title} leaks any user's secret_content #69)Mass Assignment / Privilege Escalation at registration (API6:2019) —
api_views/users.pyregister_user(): no longer reads any admin/privilege field from client input; new accounts are always created non-admin. (was PR Fix Mass Assignment: block client-set admin flag on self-registration #72)Excessive Data Exposure via debug endpoint (API3:2019) —
api_views/users.pydebug()now requires a valid bearer token (401) and admin privileges (403) before returning data;models/user_model.pyjson_debug()no longer serializes the plaintext password field at all. OpenAPI spec updated to document auth requirements. (was PR Fix: Excessive Data Exposure on GET /users/v1/_debug (Challenge 5) #71)User and Password Enumeration on login (API2:2019) —
api_views/users.pylogin_user(): returns a single generic "Username or Password Incorrect!" message for both a nonexistent username and a wrong password, removing the enumeration oracle. (was PR Fix username/password enumeration on login (API2:2019) #67)ReDoS in email update (API4:2019) —
api_views/users.pyupdate_email(): replaced the catastrophic-backtracking nested-quantifier regex with a bounded-repetition equivalent plus a 254-character length cap. (was PR Fix ReDoS in email update regex (challenge 7, API4:2019) #70)Lack of Rate Limiting on login (API4:2019) —
api_views/users.pylogin_user(): added an in-memory lockout keyed by (client IP, attempted username) — 5 failed attempts within a 60s window returns 429 withRetry-Afteruntil the window expires; a successful login clears the counter; unrelated accounts/IPs are unaffected. (was PR Fix Lack of Rate Limiting on login (API4:2019, Challenge 8) #73)Weak/hardcoded JWT signing key (API2:2019) —
config.py: removed the hardcodedSECRET_KEY = 'random'literal; the key is now sourced from aSECRET_KEYenvironment variable or a securely-generated per-process random key, so tokens can no longer be forged offline using the old known key. (was PR Fix: remove hardcoded JWT signing key (challenge 9) #68)All fixes were independently re-verified together on this merged branch by running the full app locally (WSL,
vulnerable=1, seeded via/createdb) and confirming each original exploit now fails while every corresponding legitimate feature (login, registration, password change, email update, book retrieval, debug access for real admins, JWT auth) still works correctly.