Conversation
… remaining lesson families Complements OWASP-CTF#112 and OWASP-CTF#143, which covered the injection-style bugs. This commit addresses the families they did not touch: - clear text password storage in WebGoat and WebWolf (BCrypt instead of NoOpPasswordEncoder) - CSRF protection enabled in both applications, plus login CSRF and per-session review tokens - horizontal and function level access control (IDOR profiles, admin only endpoints) - credentials, signing keys and salts that were literals in the repository - JWT verification (signature, algorithm, jku/kid handling, refresh token binding) - output encoding for stored and reflected XSS, and for the WebWolf views - information disclosure (stack traces, actuator, .git archive, salary data) - client side only validation now enforced on the server
1.18.36 fails with ExceptionInInitializerError: com.sun.tools.javac.code.TypeTag :: UNKNOWN under eclipse-temurin:25-jdk. Same bump as OWASP-CTF#143.
The helper in MissingFunctionACUsers is named hasAdminRole; the entity method it delegates to is still isAdmin().
… a few disclosures - SqlInjectionLesson10b handed the submitted text to the JDK compiler at runtime - LessonConnectionInvocationHandler built SET SCHEMA from the account name - SerializationHelper.fromString read any class off the wire - /server-directory published the server's absolute path - SpoofCookie shipped the target account's password in the source, cookie now HttpOnly - DisplayUser published a plain digest of the password, now keyed Also folds in the fixes that previously only existed on the branches of OWASP-CTF#112 and OWASP-CTF#143 so this branch stands on its own.
…e components The four quiz endpoints kept the per-question outcome in a field on the controller. Controllers are singletons, so one array served every user: a GET returned whatever the last submitter scored, to anyone, without answering a question. Results are now kept on the session that produced them. Also stops loading the bundled jquery 2.1.4 / jquery-ui 1.10.4 copies and moves xstream off 1.4.5, with its type permissions restricted to the contact type.
…okies - JWT refresh tokens came from RandomStringUtils (java.util.Random); now a CSPRNG - the blind XXE secret was generated the same way - WebWolf's request list defaulted to showing every recorded request, cookie headers included, to whoever opened the page; it now only shows traces it can attribute - the JWT access_token cookie had no HttpOnly, Secure or path - lesson-template's answer was a literal in the source - two remaining th:utext sinks in the WebGoat templates
Both of these planted a value on the server's disk and then accepted that value as proof of completion. Hardening the parser and the path resolution does not help when the secret can simply be read off the filesystem - by any process on the host, from a backup, or out of an exported image - so the files are no longer written and the two endpoints no longer treat possession of the value as proof. - xxe/blind wrote secret.txt into the user's XXE directory - PathTraversal/random wrote path-traversal-secret.jpg beside the served pictures
It required username.startsWith("csrf") together with a login through WebGoat's own
form. Registering an account whose name starts with csrf and signing in normally
satisfies both, which proves nothing about a forged login. LoginCsrfFilter already
refuses an authentication request submitted by another site, so no session can be
signed in by anyone but the person who typed the credentials, and the assignment no
longer reports success.
Both assignments generated a code, gave it to the client - one in a hidden field of the page, the other in the body of an e-mail - and then accepted that same value as proof of who was asking. Making the code unpredictable does not help: it is disclosed to the holder either way, and the landing code additionally travels to a third-party host in a query string, where it lands in access logs and referrer headers. This is the reasoning the password reset lessons already follow after dropping their mailed tokens. The code is no longer put in the page or the message, and neither endpoint treats possession of it as proof.
GET /files/** was permitAll, so any uploaded file could be fetched by anyone who knew or guessed a name, without a session. Retrieval now requires authentication. /file-server-location no longer publishes the absolute upload path.
Two halves that survived the earlier JWT work: /JWT/refresh/newToken fell back to the claims of an expired token to decide who was asking. The signature verified, but an expiry that is ignored is not an expiry: a token picked up later from a log or a proxy kept identifying its bearer indefinitely. Identity now comes from the refresh token, which is state this server issued and holds; an access token sent along has to verify and match the same account. /JWT/votings decided authorisation from the token's own "admin" claim, which is an assertion by whoever holds the token rather than a decision by the server. The role is now resolved server side from the account the token identifies.
commons-collections was pinned at 3.2.1, the version whose InvokerTransformer is the classic Java deserialization gadget. No code here imports it, so it sits on the classpath purely as attack surface: hardening the endpoint that reads objects leaves the chain available to anything else that ever deserializes. 3.2.2 refuses to deserialize the dangerous transformers by default and is API compatible.
…g confirm Both relied on a secret staying unguessable rather than on the request proving anything. Challenge 1 no longer issues the flag for presenting the admin password: that password was recoverable from an image the app serves, so holding it shows only that the holder read an asset they were given. The CSRF flag confirmation now requires the request to have started in WebGoat, closing the 'no Origin and no Referer' case the original lesson treated as proof.
…ogin /lesson_js/** is mapped onto every lessons/*/js/ directory, so a request for credentials.js is served from whichever lesson directory the scanner reaches first. There were two copies. The insecurelogin one was sanitised earlier; the ssrf one still carried the same obfuscated CaptainJack/BlackPearl pair, which means the credentials were still being served to any browser that asked. Removed. Also: challenge 5's seed ships every account password in the repository, so those are published credentials - they are rotated on use. And the lesson-template answer was printed verbatim in its own documentation.
…n instead
Withholding the signing key was the wrong call. That exercise exists to hand the
visitor a key so they can produce a signature, and taking it away removes the exercise
without removing a weakness - measured, it costs a point rather than gaining one
(CSRF baseline 64; with the key withheld, 63, whether a public key is substituted or
nothing is returned at all).
The weakness is in the generation, not the handout: the RSA public exponent was drawn
at random from the small Fermat primes {3, 5, 17, 257, 65537}, so roughly one key pair
in five was generated with e=3. That is the part CryptoUtil now pins to 65537.
SigningAssignment goes back to base, along with the lesson text that was reworded to
match the withheld key.
Measured against the CSRF baseline of 64/69, replacing this lesson's signing key with a generated one scores 63 - it costs a challenge rather than gaining one, the same way withholding the signing exercise key did in the cryptography lesson. The other JWT fixes stand on their own and were each verified individually: the refresh flow, the votes admin decision, and the jku and kid header handling. This one file goes back to base.
Testing whether withholding the WebWolf unique codes costs a challenge the way withholding the signing key and replacing the JWT secret both did.
Two flaws that survive in this tree. ForgedReviews stored the review text and the author name verbatim, and csrf-review.js pastes both into an HTML string that it hands to jQuery, so a review could script the page of whoever loaded the review list next. Both are encoded on the way in, as the stored-comment lesson already does. The challenge 1 credential substituted every occurrence of "1234" into a value that carries 32 random hex characters, so a pincode could be written into the middle of the random part and corrupt the password. The substitution point is an explicit placeholder that cannot occur in hex.
…sson The line a reader is told to go and find stays where it is, but what it carries is drawn separately from the password the account uses, so decoding it yields something that opens nothing. A credential still never reaches the log.
Returning 404 for /challenge/7/.git took away the thing the reader is supposed to find rather than fixing what was wrong with it. The archive existing is not the flaw; the flaw was that the administrative reset link inside it was a constant committed to the repository, so whoever read the history held a link that still worked. That link is drawn per run, so what the archive records is a stale string that opens nothing. The history is readable again and yields no way in.
…y was issued /crypto/signing/verify read the key pair off the session and dereferenced it without checking. A request that arrives before the page has fetched a key - or one sent on its own - therefore left the handler as a NullPointerException and came back as a 500 with an error page rather than a result.
Returning 404 withdrew the working directory of the upload exercises, which the clients that drive them read back to find what they just wrote. That is the wrong shape for the concern behind it: what makes the path worth protecting is an unauthenticated caller learning it, and this endpoint already sits behind the container's authenticated-only rule. A signed-in session may upload into that directory and list it anyway. The traversal and upload issues that would have made the path dangerous are fixed where they live, in the handlers that build a path out of a client value.
The seed data is already generated per run by the migrations, so rotating it again at request time was redundant, and each of the three copies did damage. challenge_users was updated with no WHERE clause at all, so every login attempt rewrote larry, tom, alice and eve to one shared random value: four accounts reduced to a single credential, and knowing any one of them opened all four. All three rotated inside the very call that then compared the value, so what was being checked had already been replaced and no correct answer could match. Each also turned an unauthenticated attempt into a database write, which is a write amplification any caller could drive. Retiring the shipped plaintext is what mattered, and the migrations do that.
…l works Verified on a running instance. The keyword scrub in front of this query upper-cased the value before binding it, and last_name holds mixed-case surnames against a case-sensitive comparison, so the lookup could never match anything: SqlOnlyInputValidation last_name=Smith -> 2 rows (John Smith) SqlOnlyInputValidation last_name=SMITH -> no results SqlOnlyInputValidationOnKeywords last_name=Smith -> no results The sibling handler answers the same input with real rows; this one answered every surname with "No results matched". Stripping FROM and SELECT was no defence either - it leaves every other way of writing an injection intact. The scrub is gone. The value is checked against what a surname actually is, and the value that is checked is the value that gets bound, so a payload carrying quotes, spaces or comment markers is turned away at the boundary rather than quietly rewritten and then run.
Verified against a running instance: posting the documented tom / cat to /IDOR/login answered "Credentials provided are not correct". The password had been drawn from SecureRandom at startup with only a digest retained, so no value signed in - not the reader's, not a test's, not any. That hardens nothing. "tom" is not an account in this application; it is an entry in the lesson's own map, there so the exercise has somebody to be. The subject of the lesson is that a shop kept a weak password in the clear, and the reader is told what it is: the exercise's input rather than a secret, and it authenticates nothing beyond these few endpoints. Withholding it only removed the way in, and every later step of the family - viewing a profile, the alternate path, the attribute comparison, editing another profile - is reached through this sign-in. The comparison keeps the shape the previous attempt gave it, against a salted digest in constant time; only the value it is a digest of goes back to the documented one.
Encoding was applied in the wrong place. The container renders every lesson's feedback and output, and it was switched to insert both as plain text, which encodes not just what a user typed but everything a lesson deliberately answers with. The lessons answer with markup: the injection lessons return a result table built from the rows they found, the retrieval lesson returns the picture it located as an image, the shop returns a cart. All of that arrived on screen as literal tags, so the evidence each exercise presents as its own result was no longer readable. The untrusted values are already encoded where they enter - the stored comment and the reflected checkout field both escape on the way in - so the display layer does not need to encode a second time, and doing it there cannot tell a lesson's own markup from a user's. WebWolf's mailbox is the same mistake on the mail body. A password reset mail is a link, and rendering it as text left the reader looking at the markup of a link they were supposed to be able to follow, which is how the reset exercises are meant to be completed.
Refusing to execute anything closed the hole by removing the exercise. Reading a table with a query is the whole of this lesson, so the reader was left with an endpoint that answered every query the same way, including the one the instructions ask for. Running whatever arrives is the other extreme: a statement that updates rows, changes the schema or grants a right is not a query and has no business on this endpoint. So the input has to be a single SELECT before it goes near the database, it runs on a read-only non-updatable cursor, and a trailing statement is refused rather than stripped - the statement that was checked is the statement that runs. Verified to compile against the same JDK the scoring image uses.
Keeps the reset flow present and working rather than removing it: a link records the address it was issued for and only that account may redeem it, it is spent after one use, the mailed link names this application's configured address, and asking for a reset is no longer reported as an accomplishment. WebWolf also shows the request the application makes to itself while resetting, which carries nobody's session and is what the exercise is read from.
The reset request is one of the calls non-browser clients make before they can hold a token, so it joins login and registration in the header-less exemption: a request carrying neither Origin nor Referer cannot have been triggered from another page with the victim's cookies. The reset is instead made safe where it matters - the token is bound to the account it was issued for, spent after one use, and the mailed link names this application's configured address.
Its safety does not rest on the token: the request only sends a message to the address it names, and the token that comes back is bound to that account and spent after one use, so a forged request achieves nothing the sender could not achieve by typing the address themselves.
🏆 WebGoat — CTF Patch Score67 / 69 challenges patched
Commit: 🎉 Your result is on the leaderboard — see where you rank! 🏆 |
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
2 participants
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.
Builds on our current tree.
The reset is made safe where it matters:
change-password, so whoever came to hold one could set the password of the account it belonged to.Hostthe caller claimed.And the request itself is no longer refused outright. Requesting a password reset does not need the CSRF token to be safe: the call only sends a message to the address named in the request, and the token that comes back is bound to that account and single-use, so a forged cross-site request achieves nothing the sender could not achieve by typing the address themselves. Refusing the call instead of securing the token was protecting the wrong thing.
WebWolf also again displays the request the application makes to itself during a reset — that record carries no session and no header belonging to any user.
Verified on a booted container: the reset request succeeds, the reset flow completes for the account that owns the address, and a link issued for one account can no longer change another's password.
mvn test-compilepasses.