Conversation
…jection) UrlAccess3.java granted the super-admin result key to any authenticated player who simply Base64-encoded the literal string 'MrJohnReillyTheSecond' into the client-controlled 'currentPerson' cookie - the servlet only checked that the (attacker-supplied) cookie value matched a hardcoded name, never verifying the caller actually holds a privileged server-side session. Fixed by additionally requiring Validate.validateAdminSession(ses) before honoring the super-admin cookie claim, mirroring the access-control gate already used for UrlAccess1Admin/UrlAccess2Admin. A caller who forges the cookie without a real admin session now gets the same generic 'invalid user' response as any other tampered value. UrlAccess3UserList.java built its lookup query by concatenating the same attacker-controlled cookie value directly into a SQL string (SELECT userName FROM users WHERE userRole = "admin" OR userName = "<cookie>"), letting an attacker inject arbitrary SQL via the cookie to enumerate the full user directory (including the super admin's real name) beyond what the intended admin-role listing exposes. Fixed by binding the value through a PreparedStatement parameter instead of string concatenation, preserving identical behavior for legitimate values. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🏆 Security Shepherd — CTF Patch Score1 / 40 challenges patched
Commit: 🎉 Your result is on the leaderboard — see where you rank! 🏆 |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The prior commit still let a forged 'currentPerson' cookie reach the super-admin branch as long as the caller also held a real Shepherd admin session. On reflection there is no legitimate way for a normal player session to ever hold that role for this level, and the 'currentPerson' cookie is set by plain client-side JavaScript with no server-side issuance or signing whatsoever - it is not a credential of any kind, privileged or otherwise. Gating the branch behind an extra condition still implied the cookie's claim carried some authority. UrlAccess3.java: removed the super-admin branch outright. The cookie is still decoded and logged (so genuine 'no change' / tampered-cookie diagnostics keep working), but no cookie value - including the exact super-admin name - can produce anything beyond the same generic 'invalid user' response given to any other tampered value. UrlAccess3UserList.java: stopped reading the 'currentPerson' cookie altogether. The directory lookup now always queries a single hardcoded public entry, and the query's 'OR userRole = admin' clause (which unconditionally disclosed every admin-role account name to any logged in user) has been dropped along with it. This removes the SQL injection surface completely (no request-derived data reaches the SQL string) and stops the endpoint from disclosing privileged usernames at all, not just via injection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Fixes Challenge-34-URL-Access-3 (Failure to Restrict URL Access 3), category A01.
Vulnerability
UrlAccess3.javagranted the super-admin result key to any authenticated player who simply Base64-encoded the literal stringMrJohnReillyTheSecondinto the client-controlledcurrentPersoncookie. The cookie is set by plain client-side JavaScript (document.cookie = ...) with no server-side issuance or signing whatsoever — it carries no authority at all, yet the servlet treated a match against it as sufficient to hand out the privileged result key. This is a textbook failure to restrict access to privileged functionality: the authorization decision was made entirely from client-supplied, unverified state.UrlAccess3UserList.java(the companion endpoint used to discover valid usernames, reachable via JS in the level's JSP) built its lookup query by concatenating that same attacker-controlled cookie value directly into a SQL string:SELECT userName FROM users WHERE userRole = "admin" OR userName = "<cookie>";— letting an attacker inject arbitrary SQL via the cookie, and even without injecting anything, unconditionally disclosing every admin-role account name (a hint toward the real super admin identity) to any logged-in user.Fix
UrlAccess3.java: removed the super-admin branch outright rather than gating it behind an extra check. There is no legitimate, server-verified way for any request to actually be the sub-schema's super admin, so that response is now simply unreachable — anycurrentPersoncookie value other than the plain guest one (including the exact super-admin name) produces the same generic "invalid user" response as any other tampered value. The cookie is still decoded and logged for visibility, but it can no longer unlock anything.UrlAccess3UserList.java: stopped reading thecurrentPersoncookie entirely. The directory lookup now always queries a single hardcoded public entry via a parameterizedPreparedStatement, and theOR userRole = adminclause that unconditionally disclosed privileged accounts has been removed along with it. This eliminates the SQL injection surface completely (no request-derived data reaches the SQL string) and stops the endpoint from disclosing privileged usernames at all.Verification
javac, andmvn -Pdocker clean install(includingspotless:check) reaches BUILD SUCCESS locally.currentPerson=MrJohnReillyTheSecondcookie now returns the generic "invalid user" response with no key (previously returned the key), confirmed a SQLi-style payload in the cookie against the UserList endpoint no longer alters the query, and confirmed the level's unrelated legitimate/red-herring response paths (guest cookie, correct/incorrect red-herring params) are unchanged.