Conversation
…L1-4 (parameterized queries), open redirect L1-4 (same-origin allowlist) - CryptographicFailures LEVEL_9: switch from unsaltedSha256Hex to a per-entry random salt stored as "salt:hash", verified via the existing PasswordHashingUtils.isValidSaltedSha256 helper, closing the rainbow-table/ identical-hash weakness (CWE-759/CWE-326). - CryptographicFailures LEVEL_10: replace reversible AES-128 encryption keyed by the password itself with a one-way bcrypt digest (same pattern as the existing secure LEVEL_11), so the stored value can never be decrypted even if guessed (CWE-326). - ErrorBasedSQLInjectionVulnerability LEVEL_1-4: bind the id query parameter via PreparedStatement placeholders instead of string concatenation (LEVEL_4 previously built a PreparedStatement but still concatenated the value into the SQL text itself, so it was not actually parameterized). - Http3xxStatusCodeBasedInjection LEVEL_1-4: replace the per-level ad hoc prefix blacklist (http/https/www/'//'/NUL) with a single same-origin allowlist check that also rejects any embedded control character (tab, CR, LF, NUL), closing the ftp:// scheme bypass and the browser URL- normalization bypass while still allowing same-application relative paths and same-origin absolute URLs through. - Updated the corresponding JUnit tests that previously asserted the vulnerable behavior as passing test cases. Verified locally: gradle compileJava/compileTestJava/test all pass (40/40 + 1 skipped pre-existing test in Http3xxStatusCodeBasedInjectionTest, 5/5 in ErrorBasedSQLInjectionVulnerabilityTest). Booted the app via bootRun and manually confirmed each of the 10 targeted exploits (SQLi OR-injection on L1-4, ftp/protocol-relative/bare-domain/absolute-external open redirect on L1-4, and the crypto storage endpoints) now fails while the legitimate lookup/redirect/challenge-response behavior still works. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🏆 VulnerableApp — CTF Patch Score4 / 110 challenges patched
Commit: 🎉 Your result is on the leaderboard — see where you rank! 🏆 |
…llowlist (L1-4) Levels 1-4 previously validated the returnTo target with a hand-rolled same-origin URI check (scheme+authority match, or a leading single slash). That check still let a value like "/\evil.com" through because it only rejected a literal "//" or "\" prefix, not a backslash appearing after a leading slash - a shape browsers normalize to a protocol-relative "//evil.com" redirect. Replaced it with the fixed allowlist already used by this same class's secure Level 8/11 implementations (WHITELISTED_URLS::contains, i.e. exactly "/" or "/VulnerableApp/"). This is also the only value the legitimate challenge UI ever sends (confirmed in the LEVEL_1 JS template), so no legitimate use case is affected. Updated Http3xxStatusCodeBasedInjectionTest to match: dropped the RequestEntity-based origin-matching tests (no longer relevant) and added direct allowlist tests per level (whitelisted path still redirects; external, ftp scheme, bare-domain, and null-byte payloads are all rejected).
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 10 challenges:
salt:hash) verified via the existingPasswordHashingUtils.isValidSaltedSha256, closing the unsalted/rainbow-table weakness. LEVEL_10 replaces reversible AES-128 encryption keyed by the password itself with a one-way bcrypt digest (same pattern as the existing secure LEVEL_11), so the stored value can never be decrypted even if guessed.idquery parameter is now bound via PreparedStatement placeholders instead of string concatenation (LEVEL_4 previously built a PreparedStatement object but still concatenated the value into the SQL text itself, so it was not actually parameterized).URI-parsing check, but that check still had a bypass: it only rejected a literal//or\prefix, so a value like/\evil.comslipped through (browsers normalize an embedded backslash to a second slash, turning this into the protocol-relative//evil.com). Levels 1-4 now use a fixed allowlist instead (WHITELISTED_URLS::contains, i.e. exactly/or/VulnerableApp/) — the same pattern already used by this class's own pre-existing secure Level 8/11 implementations, and the only value the legitimate challenge UI ever sends (confirmed in the LEVEL_1 JS template).Updated the corresponding JUnit tests that previously asserted the vulnerable behavior as passing cases.
Local verification (WSL, Java 17, gradle):
compileJava,compileTestJava, andtest(targeted at the two modified test classes) all pass: 33/33 non-skipped inHttp3xxStatusCodeBasedInjectionTest, 5/5 inErrorBasedSQLInjectionVulnerabilityTest.bootRunand manually curl-verified:id=1still returns the Audi record on all 4 levels;id=x' OR '1'='1now fails with a data-conversion error instead of returning data.returnTo=/VulnerableApp/still 302-redirects on all 4 levels;returnTo=https://evil.com,ftp://ftp.dlptest.com/, a bareevil.com, and a%00-prefixed payload all now return 200 with no Location header.aa123456) is confirmed accepted; the underlyingisValidSaltedSha256/isValidBcryptround-trip is covered by existing passing unit tests (PasswordHashingUtilsTest).🤖 Generated with Claude Code