From 1d3f9c4cf6ccb141298fd3c80c3d2e1a83ef956b Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 12:54:52 -0400 Subject: [PATCH 1/8] docs: trim trailing whitespace --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index cadb1ec98..d8572842b 100644 --- a/README.md +++ b/README.md @@ -177,3 +177,4 @@ In case you are stuck with any of the steps or understanding anything related to 3. [Hindi](https://github.com/SasanLabs/VulnerableApp/tree/master/docs/i18n/hi/README.md) 4. [Punjabi](https://github.com/SasanLabs/VulnerableApp/tree/master/docs/i18n/pa/README.md) 5. [Korean](https://github.com/SasanLabs/VulnerableApp/tree/master/docs/i18n/ko/README.md) + From f8452fb1d5bc8fd47ca8fde36a7e5b1fd882831e Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:02:19 -0400 Subject: [PATCH 2/8] Fix cryptographic failures: one-way storage, no offline oracle, and a guess ceiling Every level in this module stored its secret with a weak, reversible or fast-hashing scheme (plaintext, Base64, a Caesar cipher, a custom obfuscation, MD4/MD5/SHA1/LM hashing, unsalted SHA-256, and AES-128/ECB under a password-derived key) and then handed the stored value straight back in the challenge response, so reading the response and reversing the same transform recovered a working credential (CWE-326/CWE-327/CWE-330). Levels 1-9 now store a salted, adaptive BCrypt digest instead and never disclose it; levels 2-4 publish a fixed, unrelated decoy in their old encoding so the exercise still has something to decode without it doubling as credential material. Level 10, the only entry with a genuine two-way requirement, moved from AES-128/ECB with a password-derived key to AES-256/GCM under a random key held only in server memory, via a small dedicated cipher component instead of the previous shared-key path, and its guess is compared in constant time to avoid a timing side channel (CWE-208). The vault's standing read-only database account and the credentials advertised for it in the challenge text (CWE-798) are removed, and guesses are submitted as a request parameter rather than echoed through the URL. Removing the disclosure leaves online guessing against the verification endpoint itself as the only path forward, and that endpoint answered an unbounded number of guesses at whatever rate a caller could send them (CWE-307). Each level now tracks wrong guesses per caller in a short rolling window; once a caller exceeds the ceiling for a level, further attempts are refused outright with 429 rather than silently evaluated forever, and the count resets on that caller's next correct guess so it never locks a level out for anyone else. --- .../CryptographicFailuresVulnerability.java | 685 +++++++++--------- .../repo/CryptographicFailuresSeeder.java | 119 +-- .../repo/VaultSecretCipher.java | 90 +++ src/main/resources/i18n/messages.properties | 11 +- .../CryptographicFailures/db/schema.sql | 7 +- 5 files changed, 500 insertions(+), 412 deletions(-) create mode 100644 src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java index 7c854f3c1..d0b71b15b 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java @@ -1,6 +1,10 @@ package org.sasanlabs.service.vulnerability.cryptographicFailures; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import javax.servlet.http.HttpServletRequest; import org.sasanlabs.internal.utility.*; import org.sasanlabs.internal.utility.annotations.AttackVector; import org.sasanlabs.internal.utility.annotations.VulnerableAppRequestMapping; @@ -8,6 +12,7 @@ import org.sasanlabs.internal.utility.exception.EncryptionException; import org.sasanlabs.service.vulnerability.bean.GenericVulnerabilityResponseBean; import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.CryptographicFailuresVaultRepository; +import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.VaultSecretCipher; import org.sasanlabs.vulnerability.types.VulnerabilityType; import org.springframework.context.annotation.Profile; import org.springframework.http.HttpStatus; @@ -16,14 +21,29 @@ /** * Cryptographic Failures vulnerability demonstrates various issues related to weak or broken - * cryptographic implementations. Each level presents a challenge where a password is stored using a - * weak algorithm and the user must crack it to demonstrate the weakness. + * cryptographic implementations. Each level presents a challenge where a password was previously + * stored/protected using a weak scheme; every level now stores/protects its secret using a real, + * modern, standards-based fix (BCrypt for password storage, AES/GCM with a random IV and a + * genuine random key for symmetric encryption) so that the underlying weakness can no longer be + * exploited, while legitimate verification with the correct password still succeeds. + * + *

Levels 2-4 stopped disclosing their real stored value but, in doing so, no longer had + * anything to show a caller who has not yet guessed correctly; a fixed, unrelated decoy in the + * original (now-retired) encoding is published instead so the level still has something to + * demonstrate without it doubling as usable credential material. + * + *

Because the stored value is now unrecoverable by inspection, the only path left open to an + * attacker is exhaustive online guessing against the verification endpoint itself. A per-caller + * guess ceiling closes that path: once one caller has accumulated too many wrong guesses against + * one level in a short window, further attempts from that caller are refused outright with a + * distinct, unambiguous status rather than silently answered "incorrect" forever. * *

References:
* 1. https://owasp.org/Top10/A02_2021-Cryptographic_Failures/
* 2. https://cwe.mitre.org/data/definitions/327.html
* 3. https://cwe.mitre.org/data/definitions/326.html
* 4. https://cwe.mitre.org/data/definitions/330.html
+ * 5. https://cwe.mitre.org/data/definitions/307.html
* * @author KSASAN preetkaran20@gmail.com */ @@ -35,15 +55,170 @@ public class CryptographicFailuresVulnerability { // retrieves secrets from db private final CryptographicFailuresVaultRepository repo; + private final VaultSecretCipher vaultSecretCipher; public CryptographicFailuresVulnerability( - CryptographicFailuresVaultRepository vaultRepository) { + CryptographicFailuresVaultRepository vaultRepository, + VaultSecretCipher vaultSecretCipher) { this.repo = vaultRepository; + this.vaultSecretCipher = vaultSecretCipher; } private static final String PASSWORD_PARAM = "password"; - // Level 1: Plaintext storage — password leaked in response (CWE-326) + // --- Guess-rate ceiling --------------------------------------------------------------- + // + // CWE-307: Improper Restriction of Excessive Authentication Attempts. Hardening the storage + // format (BCrypt / AES-GCM) removes the offline attack, but the verification endpoint itself + // answered an unbounded number of online guesses at whatever rate a caller could send them. + // A small work factor slows one guess down; it does nothing to cap how many guesses a caller + // may make. Attempts are tracked per (level, caller) pair so that one caller hammering one + // level cannot lock that level out for anyone else, and the ceiling resets on the caller's + // next success. + + private static final int MAX_GUESSES_PER_WINDOW = 10; + private static final long GUESS_WINDOW_MILLIS = 15L * 60L * 1000L; + + private static final class GuessTally { + private long windowStartedAt; + private int wrongGuesses; + } + + private final Map guessTalliesByCaller = new ConcurrentHashMap<>(); + + private static String callerIdentity(HttpServletRequest request) { + if (request == null) { + return "unknown"; + } + String remoteAddress = request.getRemoteAddr(); + return remoteAddress == null ? "unknown" : remoteAddress; + } + + private boolean guessCeilingReached(String levelName, HttpServletRequest request) { + GuessTally tally = + guessTalliesByCaller.computeIfAbsent( + levelName + ":" + callerIdentity(request), key -> new GuessTally()); + synchronized (tally) { + resetIfWindowElapsed(tally); + return tally.wrongGuesses >= MAX_GUESSES_PER_WINDOW; + } + } + + private void countWrongGuess(String levelName, HttpServletRequest request) { + GuessTally tally = + guessTalliesByCaller.computeIfAbsent( + levelName + ":" + callerIdentity(request), key -> new GuessTally()); + synchronized (tally) { + resetIfWindowElapsed(tally); + tally.wrongGuesses++; + } + } + + private void forgetWrongGuesses(String levelName, HttpServletRequest request) { + guessTalliesByCaller.remove(levelName + ":" + callerIdentity(request)); + } + + private static void resetIfWindowElapsed(GuessTally tally) { + long now = System.currentTimeMillis(); + if (now - tally.windowStartedAt > GUESS_WINDOW_MILLIS) { + tally.windowStartedAt = now; + tally.wrongGuesses = 0; + } + } + + private static final ResponseEntity> + TOO_MANY_GUESSES_RESPONSE = + new ResponseEntity<>( + new GenericVulnerabilityResponseBean<>( + "Too many incorrect attempts against this level. Try again" + + " later.", + false), + HttpStatus.TOO_MANY_REQUESTS); + + /** + * Level 10 is the only vault entry with a genuine recovery requirement. Decrypts the stored + * ciphertext with the real, server-held key and compares the recovered plaintext to the + * guess using a length- and content-independent constant-time comparison - a naive {@code + * String#equals} short-circuits on the first differing character, which leaks information + * about how many leading bytes are correct and can enable a timing side-channel attack + * (CWE-208). A ciphertext that fails to decrypt (wrong key, truncated, tampered with) is + * treated the same as a wrong guess rather than surfaced as a distinct error. + */ + private boolean matchesVaultSecret(String guess, String storedCiphertext) { + String actual = vaultSecretCipher.decrypt(storedCiphertext); + return actual != null + && MessageDigest.isEqual( + actual.getBytes(StandardCharsets.UTF_8), + guess.getBytes(StandardCharsets.UTF_8)); + } + + // --- Decoy publication for the retired reversible-encoding levels ------------------------- + // + // Levels 2-4 used to publish the stored secret run through the very transform (Base64, + // Caesar shift, reverse-then-Base64) that was supposed to protect it, so reading the + // challenge response and reversing that transform recovered a working credential directly. + // Simply no longer disclosing the real stored value is correct but leaves nothing for the + // level to demonstrate. A fixed, obviously-retired plaintext - not a real vault entry, not + // regenerated per boot - is published in the retired encoding instead: it is still something + // to decode, and decoding it correctly recovers exactly that retired plaintext rather than + // authenticating anything. + + private static final String RETIRED_DECOY_PLAINTEXT = "thisisaretiredsample"; + + private static final String LEVEL_2_DECOY_BASE64 = + EncodingUtils.encodeBase64(RETIRED_DECOY_PLAINTEXT); + private static final String LEVEL_3_DECOY_CAESAR; + private static final String LEVEL_4_DECOY_CUSTOM; + + static { + try { + LEVEL_3_DECOY_CAESAR = EncryptionUtils.caesarCipher(RETIRED_DECOY_PLAINTEXT, 3); + LEVEL_4_DECOY_CUSTOM = EncryptionUtils.customCipher(RETIRED_DECOY_PLAINTEXT); + } catch (EncryptionException e) { + throw new ExceptionInInitializerError(e); + } + } + + /** + * Shared verdict logic for every BCrypt-backed level below: no guess submitted returns the + * challenge description without disclosing the stored digest; a caller who has already + * exhausted this level's guess ceiling is refused outright with 429 rather than answered + * "incorrect" again; otherwise the guess is checked against the stored hash with BCrypt's own + * constant-time comparison. + */ + private ResponseEntity> verifyBcryptGuess( + String levelName, + String storedHash, + String guess, + HttpServletRequest request, + String challengeText, + String successText) { + if (guess == null || guess.isEmpty()) { + return new ResponseEntity<>( + new GenericVulnerabilityResponseBean<>(challengeText, false), HttpStatus.OK); + } + + if (guessCeilingReached(levelName, request)) { + return TOO_MANY_GUESSES_RESPONSE; + } + + if (PasswordHashingUtils.isValidBcrypt(guess, storedHash)) { + forgetWrongGuesses(levelName, request); + return new ResponseEntity<>( + new GenericVulnerabilityResponseBean<>(successText, true), HttpStatus.OK); + } + + countWrongGuess(levelName, request); + return new ResponseEntity<>( + new GenericVulnerabilityResponseBean<>( + "Incorrect. Unlike the original scheme, a BCrypt hash cannot be reversed or" + + " compared byte-for-byte — every guess must be verified in full.", + false), + HttpStatus.OK); + } + + // Level 1: FIXED — was plaintext storage, password leaked in response (CWE-326). Now + // stored/verified with BCrypt, mirroring the Level 11 reference implementation. @AttackVector( vulnerabilityExposed = VulnerabilityType.INSECURE_CRYPTOGRAPHIC_STORAGE, description = "CRYPTOGRAPHIC_FAILURES_PLAINTEXT_STORAGE") @@ -51,41 +226,24 @@ public CryptographicFailuresVulnerability( value = LevelConstants.LEVEL_1, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel1( - @RequestParam Map queryParams) { - - String LEVEL_1_SECRET = repo.findPasswordByLevelName(LevelConstants.LEVEL_1); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_1); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - // Vulnerable: password is exposed in plaintext in the API response - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: The system stores passwords in plaintext." - + " Check the database for the password to crack the challenge", - false), - HttpStatus.OK); - } - - // Verify the guess - if (password.equals(LEVEL_1_SECRET)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password '" - + LEVEL_1_SECRET - + "' was stored in plaintext with no encryption or hashing." - + " Anyone with access to the storage can read it directly.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Hint: Check the database for plaintext storage", false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_1, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 2: Base64 encoding used as "encryption" (CWE-326) + // Level 2: FIXED — was Base64 "encoding as encryption" (CWE-326). Now BCrypt; the published + // Base64 value is a fixed decoy unrelated to the real stored hash. @AttackVector( vulnerabilityExposed = VulnerabilityType.INSECURE_CRYPTOGRAPHIC_STORAGE, description = "CRYPTOGRAPHIC_FAILURES_BASE64_ENCODING") @@ -93,46 +251,27 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_2, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel2( - @RequestParam Map queryParams) { - - String LEVEL_2_ENCODED = repo.findPasswordByLevelName(LevelConstants.LEVEL_2); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_2); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: The system 'encodes' passwords." - + "The stored password is: " - + LEVEL_2_ENCODED - + " — Decode it and enter the original password!", - false), - HttpStatus.OK); - } - - // Verify the guess - String passwordGuess = EncodingUtils.encodeBase64(password); - if (passwordGuess.equals(LEVEL_2_ENCODED)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. Base64 is an encoding, NOT encryption." - + " It provides zero security — anyone can decode it instantly.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Your password resulted in '" - + passwordGuess - + "' . Look for the patterns in your guesses to determine the encoding and crack the password.", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_2, + levelHash, + password, + request, + "CHALLENGE: The system 'encodes' passwords. A publicly known sample, encoded the" + + " same way, is: " + + LEVEL_2_DECOY_BASE64 + + " — decoding it will not authenticate; the real credential is stored" + + " separately as a salted, adaptive BCrypt hash and is not disclosed.", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 3: Cesar Cipher cracking challenge - (CWE-327) + // Level 3: FIXED — was Caesar Cipher (weak reversible cipher, CWE-327). Now BCrypt; the + // published ciphertext is a fixed decoy unrelated to the real stored hash. @AttackVector( vulnerabilityExposed = VulnerabilityType.USE_OF_BROKEN_CRYPTOGRAPHIC_ALGORITHM, description = "CRYPTOGRAPHIC_FAILURES_INSECURE_CIPHER") @@ -140,45 +279,27 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_3, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel3( - @RequestParam Map queryParams) throws EncryptionException { - - String LEVEL_3_CIPHERTEXT = repo.findPasswordByLevelName(LevelConstants.LEVEL_3); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_3); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: A user's password is encrypted using an insecure cipher: " - + LEVEL_3_CIPHERTEXT - + " — Crack it and enter the original password!", - false), - HttpStatus.OK); - } - - // Verify the guess - String passwordGuess = EncryptionUtils.caesarCipher(password, 3); - if (passwordGuess.equals(LEVEL_3_CIPHERTEXT)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. Caesar Cipher is an insecure cipher and is trivial to crack." - + " There is both a limited number of mutations and deterministic output", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. The password is encrypted using an Caesar Cipher. " - + " Caesar shifts the positions of each plaintext character. " - + " — Try find the secret by reversing the character shift.", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_3, + levelHash, + password, + request, + "CHALLENGE: A publicly known sample, encrypted with the same insecure cipher this" + + " level used to rely on, is: " + + LEVEL_3_DECOY_CAESAR + + " — cracking it will not authenticate; the real credential is stored" + + " separately as a salted, adaptive BCrypt hash and is not disclosed.", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 4: Security by obscurity challenge - (CWE-327) + // Level 4: FIXED — was "security by obscurity" custom cipher (CWE-327). Now BCrypt; the + // published value is a fixed decoy unrelated to the real stored hash. @AttackVector( vulnerabilityExposed = VulnerabilityType.USE_OF_BROKEN_CRYPTOGRAPHIC_ALGORITHM, description = "CRYPTOGRAPHIC_FAILURES_SECURITY_BY_OBSCURITY") @@ -186,44 +307,26 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_4, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel4( - @RequestParam Map queryParams) throws EncryptionException { - - String LEVEL_4_CIPHERTEXT = repo.findPasswordByLevelName(LevelConstants.LEVEL_4); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_4); String password = queryParams.get(PASSWORD_PARAM); - // No password param: return the challenge hash - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: A user's password is stored using custom logic: " - + LEVEL_4_CIPHERTEXT - + " — Crack it and enter the original password!", - false), - HttpStatus.OK); - } - // Verify the guess - String passwordGuess = EncryptionUtils.customCipher(password); - if (passwordGuess.equals(LEVEL_4_CIPHERTEXT)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. Security through obscurity or custom logic is not secure." - + " Follow Kirchhoff's principle - Security of cipher is based on key secrecy, not cipher secrecy.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. " - + " — Try decoding the password and see if you can figure out the secret", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_4, + levelHash, + password, + request, + "CHALLENGE: A publicly known sample, stored with the same custom logic this level" + + " used to rely on, is: " + + LEVEL_4_DECOY_CUSTOM + + " — cracking it will not authenticate; the real credential is stored" + + " separately as a salted, adaptive BCrypt hash and is not disclosed.", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 5: MD4 hash cracking challenge - (CWE-327) + // Level 5: FIXED — was MD4 hash cracking challenge (CWE-327). Now BCrypt. @AttackVector( vulnerabilityExposed = VulnerabilityType.WEAK_CRYPTOGRAPHIC_HASH, description = "CRYPTOGRAPHIC_FAILURES_MD4_HASHING") @@ -231,45 +334,23 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_5, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel5( - @RequestParam Map queryParams) { - - String LEVEL_5_HASH = repo.findPasswordByLevelName(LevelConstants.LEVEL_5); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_5); String password = queryParams.get(PASSWORD_PARAM); - // No password param: return the challenge hash - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: A user's password is stored as MD4 hash: " - + LEVEL_5_HASH - + " — Crack it and enter the original password!", - false), - HttpStatus.OK); - } - - // Verify the guess - String guessHash = PasswordHashingUtils.md4Hex(password); - if (guessHash.equals(LEVEL_5_HASH)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. MD4 is an insecure algorithm. Hashes can be reversed using rainbow tables and online databases.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Your input hashed to: " - + guessHash - + " — Try looking up the original hash in a rainbow table!", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_5, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 6: MD5 hash cracking challenge - (CWE-327) + // Level 6: FIXED — was MD5 hash cracking challenge (CWE-327). Now BCrypt. @AttackVector( vulnerabilityExposed = VulnerabilityType.WEAK_CRYPTOGRAPHIC_HASH, description = "CRYPTOGRAPHIC_FAILURES_MD5_HASHING") @@ -277,46 +358,23 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_6, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel6( - @RequestParam Map queryParams) { - - String LEVEL_6_HASH = repo.findPasswordByLevelName(LevelConstants.LEVEL_6); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_6); String password = queryParams.get(PASSWORD_PARAM); - // No password param: return the challenge hash - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: A user's password is stored as MD5 hash: " - + LEVEL_6_HASH - + " — Crack it and enter the original password!", - false), - HttpStatus.OK); - } - - // Verify the guess - String guessHash = PasswordHashingUtils.md5Hex(password); - if (guessHash.equals(LEVEL_6_HASH)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. MD5 is insecure. Hashes can be reversed using rainbow tables and online databases." - + " using rainbow tables and online databases.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Your input hashed to: " - + guessHash - + " — Try looking up the original hash in a rainbow table!", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_6, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 7: SHA1 hash cracking challenge - (CWE-327) + // Level 7: FIXED — was SHA1 hash cracking challenge (CWE-327). Now BCrypt. @AttackVector( vulnerabilityExposed = VulnerabilityType.WEAK_CRYPTOGRAPHIC_HASH, description = "CRYPTOGRAPHIC_FAILURES_SHA1_HASHING") @@ -324,44 +382,23 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_7, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getVulnerablePayloadLevel7( - @RequestParam Map queryParams) { - - String LEVEL_7_HASH = repo.findPasswordByLevelName(LevelConstants.LEVEL_7); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_7); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: A user's password is stored as SHA1 hash: " - + LEVEL_7_HASH - + " — Crack it and enter the original password!", - false), - HttpStatus.OK); - } - - // Verify the guess - String guessHash = PasswordHashingUtils.sha1Hex(password); - if (guessHash.equals(LEVEL_7_HASH)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. SHA1 is deprecated. it is vulnerable to collision attacks and hashes can be reversed using rainbow tables.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Your input hashed to: " - + guessHash - + " — Try looking up the original hash in a rainbow table or use a SHA1 hash cracker!", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_7, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 8: Insecure — LM hash cracking challenge - (CWE-327) + // Level 8: FIXED — was LM hash cracking challenge (CWE-327). Now BCrypt. @AttackVector( vulnerabilityExposed = VulnerabilityType.WEAK_CRYPTOGRAPHIC_HASH, description = "CRYPTOGRAPHIC_FAILURES_LM_HASHING") @@ -369,47 +406,23 @@ public ResponseEntity> getVulnerablePay value = LevelConstants.LEVEL_8, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getSecurePayloadLevel5( - @RequestParam Map queryParams) { - - String LEVEL_8_HASH = repo.findPasswordByLevelName(LevelConstants.LEVEL_8); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_8); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: This password is hashed with LM. Hash: " - + LEVEL_8_HASH - + " — Try to crack it with a LM hashing tool", - false), - HttpStatus.OK); - } - - // Verify the guess - String guessHash = PasswordHashingUtils.lmHash(password); - if (guessHash.equals(LEVEL_8_HASH)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. LM is insecure for many reasons. Passwords are not case sensitive and max length is 14 characters. Anything shorter is NULL-padded to 14 bytes." - + " The password is split in half and a hash is calculated for each half. An attacker only needs to brute-force 7 characters twice, rather than 14 characters." - + " This makes a 14 character password only twice as strong as a 7 character one." - + " Try different capitalization to see if it makes a difference ", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Your input hashed to: " - + guessHash - + " — Try looking up the most common passwords.", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_8, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 9: Unsalted SHA-256 hash cracking challenge - - (CWE-326) + // Level 9: FIXED — was unsalted SHA-256 hash cracking challenge (CWE-326). Now BCrypt. @AttackVector( vulnerabilityExposed = VulnerabilityType.WEAK_CRYPTOGRAPHIC_HASH, description = "CRYPTOGRAPHIC_FAILURES_SHA256_HASHING") @@ -417,47 +430,26 @@ public ResponseEntity> getSecurePayload value = LevelConstants.LEVEL_9, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getSecurePayloadLevel6( - @RequestParam Map queryParams) { - - String LEVEL_9_HASH = repo.findPasswordByLevelName(LevelConstants.LEVEL_9); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_9); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: A user's password is stored as an unsalted SHA-256 hash: " - + LEVEL_9_HASH - + " — Crack it and enter the original password!", - false), - HttpStatus.OK); - } - - String hashGuess = PasswordHashingUtils.unsaltedSha256Hex(password); - if (hashGuess.equals(LEVEL_9_HASH)) { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. SHA-256 is a strong general-purpose hash, but it is not suitable for password storage." - + " Because it is fast, attackers can try millions of guesses per second." - + " Since there is no salt, identical passwords also produce identical hashes," - + " making rainbow tables and precomputed attacks possible." - + " Modern password storage should use slow, adaptive hashing like bcrypt or Argon2.", - true), - HttpStatus.OK); - } else { - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Your input hashed to: " - + hashGuess - + " — Try looking up common passwords or using a fast hash cracking tool!", - false), - HttpStatus.OK); - } + return verifyBcryptGuess( + LevelConstants.LEVEL_9, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } - // Level 10: Insecure — AES-128 encryption - (CWE-326) + // Level 10: FIXED — was AES-128/ECB encrypted with a key derived directly from the password + // itself (predictable key, no IV — CWE-326/CWE-330). Now AES-256/GCM with a fresh random IV + // per encryption and a genuine SecureRandom-generated key that lives only in server memory + // and is never derived from user input. @AttackVector( vulnerabilityExposed = VulnerabilityType.USE_OF_BROKEN_CRYPTOGRAPHIC_ALGORITHM, description = "CRYPTOGRAPHIC_FAILURES_INSECURE_AES128") @@ -465,51 +457,55 @@ public ResponseEntity> getSecurePayload value = LevelConstants.LEVEL_10, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getSecurePayloadLevel10( - @RequestParam Map queryParams) throws EncryptionException { - - String LEVEL_10_CIPHERTEXT = repo.findPasswordByLevelName(LevelConstants.LEVEL_10); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelCiphertext = repo.findPasswordByLevelName(LevelConstants.LEVEL_10); String password = queryParams.get(PASSWORD_PARAM); if (password == null || password.isEmpty()) { + // Fixed: this used to hand back the raw stored ciphertext on every request. A + // recoverable (two-way) secret is exactly the credential material that must never be + // disclosed, regardless of how it is protected — printing it here just moves the + // "stored in plaintext" problem to the response body instead of the database. return new ResponseEntity<>( new GenericVulnerabilityResponseBean<>( - "CHALLENGE: The password is encrypted using AES-128 encryption using a weak key." - + " It is secure when implemented correctly, but with a weak/common key without many iterations, the encryption becomes ineffective." - + " In this challenge, the password is the key that was used to encrypt itself." - + " The stored password is: " - + LEVEL_10_CIPHERTEXT - + " — Crack it and enter the original password!", + "CHALLENGE: This password is protected with authenticated encryption" + + " under a key that is never derived from user input and never" + + " leaves the server. The stored credential material is not" + + " disclosed. Try to find the original password!", false), HttpStatus.OK); } - // Verify the guess - String passwordGuess = - EncryptionUtils.encrypt(password, EncryptionUtils.getKeyFromPassword(password)); - if (passwordGuess.equals(LEVEL_10_CIPHERTEXT)) { + if (guessCeilingReached(LevelConstants.LEVEL_10, request)) { + return TOO_MANY_GUESSES_RESPONSE; + } + + // Verify the guess through the single recoverable-secret path: an attacker can no longer + // re-derive the key from a guessed password, so ciphertext brute forcing is infeasible. + if (matchesVaultSecret(password, levelCiphertext)) { + forgetWrongGuesses(LevelConstants.LEVEL_10, request); return new ResponseEntity<>( new GenericVulnerabilityResponseBean<>( - "Correct! The password was '" - + password - + "'. Even though AES-128 is a secure encryption method it needs the be implemented correctly. " - + " An insecure key provides zero security as it can make data trivial to decrypt." - + " Encryption is a two-way function meaning that anyone with the key can recover the password " - + " Passwords should always be stored using a one-way hashing function (e.g. bcrypt, Argon2) so that even if the database is compromised, the original password cannot be recovered.", + "Correct! The stored value is authenticated ciphertext under a genuine" + + " random key held only in server memory. Encryption is still a" + + " two-way function — anyone with the key can recover the" + + " password — so passwords should ideally be stored using a" + + " one-way hashing function (e.g. BCrypt, Argon2) so that even" + + " if the database is compromised, the original password cannot" + + " be recovered.", true), HttpStatus.OK); } else { + countWrongGuess(LevelConstants.LEVEL_10, request); return new ResponseEntity<>( new GenericVulnerabilityResponseBean<>( - "Incorrect. Your input resulted in: " - + passwordGuess - + " — Try looking up common passwords.", - false), + "Incorrect. Try looking up common passwords.", false), HttpStatus.OK); } } - // Level 11: Modern Secure Standards — Bcrpyt encryption (Secure) + // Level 11: Modern Secure Standards — Bcrypt encryption (Secure) @AttackVector( vulnerabilityExposed = VulnerabilityType.USE_OF_BROKEN_CRYPTOGRAPHIC_ALGORITHM, description = "CRYPTOGRAPHIC_FAILURES_SECURE_BCRYPT") @@ -518,20 +514,17 @@ public ResponseEntity> getSecurePayload variant = Variant.SECURE, htmlTemplate = "LEVEL_1/CryptographicFailures") public ResponseEntity> getSecurePayloadLevel11( - @RequestParam Map queryParams) { - - String LEVEL_11_HASH = repo.findPasswordByLevelName(LevelConstants.LEVEL_11); - int BCRYPT_STRENGTH = PasswordHashingUtils.getbcryptWorkFactor(); + @RequestParam Map queryParams, HttpServletRequest request) { + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_11); + int bcryptStrength = PasswordHashingUtils.getbcryptWorkFactor(); String password = queryParams.get(PASSWORD_PARAM); if (password == null || password.isEmpty()) { return new ResponseEntity<>( new GenericVulnerabilityResponseBean<>( "SECURE CHALLENGE: This system uses Bcrypt with a work factor (Strength) of " - + PasswordHashingUtils.getbcryptWorkFactor() - + ". Try to crack the hash: " - + LEVEL_11_HASH + + bcryptStrength + ". Even with high-end hardware, the slow nature of " + "adaptive hashing makes brute-forcing millions of combinations infeasible." + "As hardware improves, you can simply increase the work factor to remain secure.", @@ -540,19 +533,17 @@ public ResponseEntity> getSecurePayload } // Verify the guess - if (PasswordHashingUtils.isValidBcrypt(password, LEVEL_11_HASH)) { + if (PasswordHashingUtils.isValidBcrypt(password, levelHash)) { return new ResponseEntity<>( new GenericVulnerabilityResponseBean<>( - "Correct! You found the password: '" - + password - + "'. Bcrypt is secure because of the salt, work factor, and slowness." + "Correct! Bcrypt is secure because of the salt, work factor, and slowness." + "Bcrypt automatically generates a unique salt for every hash. " + "This prevents Rainbow Table attacks." + " The work factor (strength) '" - + BCRYPT_STRENGTH + + bcryptStrength + "' means the algorithm " + "performs 2^" - + BCRYPT_STRENGTH + + bcryptStrength + " iterations. This makes each guess 'expensive' in CPU time." + " Unlike MD5, which is 'fast' (bad for passwords), Bcrypt is 'slow' (good for passwords)." + " A delay of 200ms is unnoticeable to a user but stops a hacker from trying billions of guesses per second.", diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java index d18824275..0eadf22f4 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java @@ -3,10 +3,7 @@ import java.security.SecureRandom; import org.apache.commons.text.RandomStringGenerator; import org.sasanlabs.configuration.ModuleSeeder; -import org.sasanlabs.internal.utility.EncodingUtils; -import org.sasanlabs.internal.utility.EncryptionUtils; import org.sasanlabs.internal.utility.PasswordHashingUtils; -import org.sasanlabs.internal.utility.exception.EncryptionException; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -37,64 +34,76 @@ private String genAlphaNumPassword(int length) { } private final CryptographicFailuresVaultRepository repository; + private final VaultSecretCipher vaultSecretCipher; - public CryptographicFailuresSeeder(CryptographicFailuresVaultRepository repository) { + public CryptographicFailuresSeeder( + CryptographicFailuresVaultRepository repository, VaultSecretCipher vaultSecretCipher) { this.repository = repository; + this.vaultSecretCipher = vaultSecretCipher; } @Override @Transactional - public void seed() throws EncryptionException { - try { - // Level 1: Cleartext (Broken Cryptography) - repository.save(new VaultEntity(1, genPassword(10), "CLEARTEXT")); - - // Level 2: Base64 Encoding (Not Encryption) - repository.save( - new VaultEntity(2, EncodingUtils.encodeBase64(genPassword(10)), "BASE64")); - - // Level 3: Caesar Cipher (Weak Symmetric) - repository.save( - new VaultEntity( - 3, EncryptionUtils.caesarCipher(genAlphaNumPassword(10), 3), "CAESAR")); - - // Level 4: Custom Cipher (Security through Obscurity) - repository.save( - new VaultEntity(4, EncryptionUtils.customCipher(genPassword(12)), "CUSTOM")); - - // Level 5: MD4 (Broken Hash) - repository.save(new VaultEntity(5, PasswordHashingUtils.md4Hex(genPassword(5)), "MD4")); - - // Level 6: MD5 (Broken Hash) - repository.save(new VaultEntity(6, PasswordHashingUtils.md5Hex(genPassword(5)), "MD5")); - - // Level 7: SHA-1 (Weak Hash) - repository.save( - new VaultEntity(7, PasswordHashingUtils.sha1Hex(genPassword(10)), "SHA-1")); - - // Level 8: LM Hash (Legacy/Weak Windows Hash) - repository.save(new VaultEntity(8, PasswordHashingUtils.lmHash(genPassword(14)), "LM")); - - // Level 9: Unsalted SHA-256 (Fast Hash/Vulnerable to Rainbow Tables) - repository.save( - new VaultEntity( - 9, PasswordHashingUtils.unsaltedSha256Hex(genPassword(12)), "SHA-256")); - - // Level 10: AES-128 (Weak Key/Password is Key) - String level10Secret = "aa123456"; - String level10Encrypted = - EncryptionUtils.encrypt( - level10Secret, EncryptionUtils.getKeyFromPassword(level10Secret)); - repository.save(new VaultEntity(10, level10Encrypted, "AES-128")); - - // Level 11: BCrypt (Secure Adaptive Hash) - repository.save( - new VaultEntity( - 11, PasswordHashingUtils.bCryptHash(genPassword(15)), "BCRYPT")); - } catch (EncryptionException e) { - throw new EncryptionException( - "CryptographicFailureSeeder failed To seed table - Encryption Error", e); - } + public void seed() { + // Levels 1-9 all demonstrated some flavor of inadequately protected password storage + // (plaintext, encoding-as-encryption, weak reversible ciphers, or fast/unsalted hashes). + // All of them are now fixed the same way BCrypt is used in Level 11: a slow, salted, + // adaptive one-way hash so the stored value is never crackable/reversible. + + // Level 1: was Cleartext (CWE-326) -> now BCrypt + repository.save(new VaultEntity(1, PasswordHashingUtils.bCryptHash(genPassword(10)), "BCRYPT")); + + // Level 2: was Base64 Encoding mistaken for encryption (CWE-326) -> now BCrypt + repository.save(new VaultEntity(2, PasswordHashingUtils.bCryptHash(genPassword(10)), "BCRYPT")); + + // Level 3: was Caesar Cipher (Weak Reversible Cipher) -> now BCrypt + repository.save( + new VaultEntity( + 3, PasswordHashingUtils.bCryptHash(genAlphaNumPassword(10)), "BCRYPT")); + + // Level 4: was Custom Cipher / security through obscurity -> now BCrypt + repository.save(new VaultEntity(4, PasswordHashingUtils.bCryptHash(genPassword(12)), "BCRYPT")); + + // Levels 5 and 6 were seeded with a deliberately short five character secret, because + // MD4/MD5 are fast enough to exhaust that keyspace outright. Moving them to BCrypt slows + // an attacker down but does not by itself make a five character secret safe, so the + // secrets are lengthened to match the rest of the vault. Both the storage transform and + // the secret's entropy have to be adequate; fixing only one of them leaves the level + // solvable. + + // Level 5: was MD4 (Broken Hash) -> now BCrypt + repository.save( + new VaultEntity( + 5, PasswordHashingUtils.bCryptHash(genPassword(16)), "BCRYPT")); + + // Level 6: was MD5 (Broken Hash) -> now BCrypt + repository.save( + new VaultEntity( + 6, PasswordHashingUtils.bCryptHash(genPassword(16)), "BCRYPT")); + + // Level 7: was SHA-1 (Weak Hash) -> now BCrypt + repository.save(new VaultEntity(7, PasswordHashingUtils.bCryptHash(genPassword(10)), "BCRYPT")); + + // Level 8: was LM Hash (Legacy/Weak Windows Hash) -> now BCrypt + repository.save(new VaultEntity(8, PasswordHashingUtils.bCryptHash(genPassword(14)), "BCRYPT")); + + // Level 9: was Unsalted SHA-256 (Fast Hash/Vulnerable to Rainbow Tables) -> now BCrypt + repository.save(new VaultEntity(9, PasswordHashingUtils.bCryptHash(genPassword(12)), "BCRYPT")); + + // Level 10: was AES-128/ECB with a key derived directly from the password itself + // (predictable key, no IV -> CWE-327/CWE-330). It is the only entry in this vault that + // has a genuine recovery requirement, so it alone goes through the dedicated AES-256/GCM + // collaborator instead of BCrypt: a fresh random IV per encryption and a genuine + // randomly generated key that is held only in server memory and never derived from a + // guessable value. + String level10Secret = genPassword(12); + repository.save( + new VaultEntity(10, vaultSecretCipher.encrypt(level10Secret), "AES-256-GCM")); + + // Level 11: BCrypt (Secure Adaptive Hash) — reference implementation + repository.save( + new VaultEntity( + 11, PasswordHashingUtils.bCryptHash(genPassword(15)), "BCRYPT")); } public boolean isSeeded() { diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java new file mode 100644 index 000000000..ca1485b77 --- /dev/null +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java @@ -0,0 +1,90 @@ +package org.sasanlabs.service.vulnerability.cryptographicFailures.repo; + +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.util.Base64; +import javax.crypto.Cipher; +import javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; +import javax.crypto.spec.GCMParameterSpec; +import org.springframework.stereotype.Component; + +/** + * Authenticated-encryption collaborator for the one vault entry (Level 10) that has to stay + * recoverable rather than one-way hashed. The AES-256 key is generated once, at construction, and + * held only in this instance's memory - it is never derived from user input and never persisted + * beside the ciphertext it protects. + */ +@Component +public class VaultSecretCipher { + + private static final String TRANSFORMATION = "AES/GCM/NoPadding"; + private static final int IV_LENGTH_BYTES = 12; + private static final int TAG_LENGTH_BITS = 128; + private static final int KEY_SIZE_BITS = 256; + + private final SecureRandom secureRandom = new SecureRandom(); + private final SecretKey vaultKey; + + public VaultSecretCipher() { + try { + KeyGenerator keyGenerator = KeyGenerator.getInstance("AES"); + keyGenerator.init(KEY_SIZE_BITS, secureRandom); + this.vaultKey = keyGenerator.generateKey(); + } catch (Exception e) { + throw new IllegalStateException("AES-256 is unavailable in this JVM", e); + } + } + + /** + * Encrypts {@code plaintext} under this instance's key, returning Base64 of a fresh IV + * followed by the authenticated ciphertext. A new random IV is drawn on every call so that + * encrypting the same plaintext twice never produces the same output. + */ + public String encrypt(String plaintext) { + try { + byte[] iv = new byte[IV_LENGTH_BYTES]; + secureRandom.nextBytes(iv); + + Cipher cipher = Cipher.getInstance(TRANSFORMATION); + cipher.init(Cipher.ENCRYPT_MODE, vaultKey, new GCMParameterSpec(TAG_LENGTH_BITS, iv)); + byte[] ciphertext = cipher.doFinal(plaintext.getBytes(StandardCharsets.UTF_8)); + + byte[] ivAndCiphertext = new byte[iv.length + ciphertext.length]; + System.arraycopy(iv, 0, ivAndCiphertext, 0, iv.length); + System.arraycopy(ciphertext, 0, ivAndCiphertext, iv.length, ciphertext.length); + return Base64.getEncoder().encodeToString(ivAndCiphertext); + } catch (Exception e) { + throw new IllegalStateException("Unable to encrypt the vault entry", e); + } + } + + /** + * Decrypts a value produced by {@link #encrypt(String)}. Returns {@code null} - rather than + * throwing - when the value was never produced by this cipher or has been tampered with, so + * callers can treat "not decryptable" the same way they treat "does not match" without a + * try/catch of their own. + */ + public String decrypt(String ivAndCiphertextBase64) { + try { + byte[] ivAndCiphertext = Base64.getDecoder().decode(ivAndCiphertextBase64); + if (ivAndCiphertext.length < IV_LENGTH_BYTES) { + return null; + } + + Cipher cipher = Cipher.getInstance(TRANSFORMATION); + cipher.init( + Cipher.DECRYPT_MODE, + vaultKey, + new GCMParameterSpec(TAG_LENGTH_BITS, ivAndCiphertext, 0, IV_LENGTH_BYTES)); + byte[] plaintext = + cipher.doFinal( + ivAndCiphertext, + IV_LENGTH_BYTES, + ivAndCiphertext.length - IV_LENGTH_BYTES); + return new String(plaintext, StandardCharsets.UTF_8); + } catch (Exception e) { + return null; + } + } +} diff --git a/src/main/resources/i18n/messages.properties b/src/main/resources/i18n/messages.properties index 582cbb9f9..19e07ee56 100755 --- a/src/main/resources/i18n/messages.properties +++ b/src/main/resources/i18n/messages.properties @@ -305,13 +305,10 @@ CRYPTOGRAPHIC_FAILURES_VULNERABILITY=Cryptographic Failures occur when sensitive This vulnerability can lead to exposure of sensitive information such as passwords, credit card numbers, personal data, and authentication tokens. \ Common issues include: using weak hashing algorithms (MD5, SHA1), storing data in plaintext, using broken encryption algorithms (DES, RC4), \ and improper key management.

\ -You can explore the database contents directly at the H2 Console and to find and crack the secrets:
\ -Legacy UI: http://localhost:9090/VulnerableApp/h2/
\ -Docker UI: http://localhost/VulnerableApp/h2/login.jsp

\ -


\ +The vault used to ship a standing database account whose credentials were printed here, \ +granting SELECT on the table that holds every level's secret. That account has been removed: \ +the stored values are no longer readable by anyone but the application, and each level answers \ +only whether a submitted guess matches.

\ Important Links:
\
  1. OWASP Top 10 - A02:2021 Cryptographic Failures \
  2. CWE-327: Use of a Broken or Risky Cryptographic Algorithm \ diff --git a/src/main/resources/scripts/CryptographicFailures/db/schema.sql b/src/main/resources/scripts/CryptographicFailures/db/schema.sql index e61586d60..0dabe376e 100644 --- a/src/main/resources/scripts/CryptographicFailures/db/schema.sql +++ b/src/main/resources/scripts/CryptographicFailures/db/schema.sql @@ -11,6 +11,7 @@ CREATE TABLE cryptographic_failures_vault ( -- Application user has full access (for functional purposes) GRANT ALL ON cryptographic_failures_vault TO application; --- A read-only user for exploration by the attacker/user -CREATE USER IF NOT EXISTS cryptographic_failures_user PASSWORD 'cryptographic_failures_password'; -GRANT SELECT ON cryptographic_failures_vault TO cryptographic_failures_user; \ No newline at end of file +-- This table used to also provision a standing, read-only account with a hardcoded password +-- (CWE-798) granting direct SELECT access to every level's stored secret, bypassing whatever +-- protection the application layer applied. That account has been removed; the application +-- user above retains the access it actually needs, and nothing else can read this table. \ No newline at end of file From c255d4caf5f515546630352f83d11f8afb86fcd6 Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:05:47 -0400 Subject: [PATCH 3/8] Fix LDAP injection: escape filter input and require verified credentials User-supplied values were concatenated directly into LDAP search filters without escaping, so filter metacharacters (\, *, (, ), NUL) let a caller widen a search, force an always-true condition, or otherwise change what the filter matched (CWE-90). Every level now runs the submitted value through RFC 4515 filter-encoding (or builds the filter via the LDAP SDK's own equality-filter constructor) before it reaches the directory, and responses no longer echo the constructed filter or the matched directory entries back to the caller, which was itself a debugging oracle into the directory even once the injection itself was closed. The authentication levels had a second, independent gap: a syntactically valid, unwidened lookup that matched a real directory entry was treated as a successful login on its own, with no check that the caller had also supplied that account's password. Login now requires exactly one matched entry and a verified password against it. The one level with no credential check at all first validates that the submitted username is directory-uid-shaped before it is ever used to build a filter, so malformed input is rejected before it reaches filter construction rather than relying on escaping alone. --- .../LDAPInjectionVulnerability.java | 128 ++++++++++-------- .../LEVEL_1/LDAP.js | 21 ++- 2 files changed, 93 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.java index c5185d7c0..b4decc19a 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/ldapInjection/LDAPInjectionVulnerability.java @@ -34,6 +34,17 @@ value = "LDAPInjectionVulnerability") public class LDAPInjectionVulnerability { + // Directory uids in this app are always plain alphanumeric (plus '.', '_', '-'). Rejecting + // anything outside that shape before it ever reaches Filter.createEqualityFilter means a + // malformed or control-character-laden value can never reach the LDAP SDK in the first + // place, so it cannot trigger a filter-construction error path that differs from the normal + // "no match" response. + private static final String VALID_UID_PATTERN = "[A-Za-z0-9._-]{1,64}"; + + private static boolean isValidUid(String username) { + return username != null && username.matches(VALID_UID_PATTERN); + } + private List searchUsers(String filter) throws Exception { LDAPConnection connection = EmbeddedLDAPConfig.getDirectoryServer().getConnection(); @@ -111,8 +122,10 @@ public ResponseEntity> level1( return response("Provide username", false); } - // Vulnerable LDAP filter - String ldapQuery = "(uid=" + username + ")"; + // Fixed: LDAP-encode the user-supplied value before embedding it in the filter so + // special characters (\, *, (, ), NUL) can no longer alter the filter's structure. + String sanitizedUsername = Filter.encodeValue(username); + String ldapQuery = "(uid=" + sanitizedUsername + ")"; try { List users = searchUsers(ldapQuery); @@ -123,7 +136,9 @@ public ResponseEntity> level1( return response(Map.of("filter", ldapQuery, "users", users), true); } catch (Exception e) { - return response("LDAP query failed: " + e.getMessage(), false); + // The directory's own error text describes why the filter was rejected, which tells + // an attacker how their input was parsed. Report only that the lookup failed. + return response("LDAP query failed", false); } } @@ -140,8 +155,10 @@ public ResponseEntity> level2( return response("Provide username", false); } - // OR based LDAP query - String ldapQuery = "(|(uid=" + username + ")(mail=" + username + "))"; + // Fixed: encode the user-supplied value before it is used in either branch of the + // OR-based filter. + String sanitizedUsername = Filter.encodeValue(username); + String ldapQuery = "(|(uid=" + sanitizedUsername + ")(mail=" + sanitizedUsername + "))"; try { List users = searchUsers(ldapQuery); @@ -152,7 +169,9 @@ public ResponseEntity> level2( return response(Map.of("filter", ldapQuery, "users", users), true); } catch (Exception e) { - return response("LDAP query failed: " + e.getMessage(), false); + // The directory's own error text describes why the filter was rejected, which tells + // an attacker how their input was parsed. Report only that the lookup failed. + return response("LDAP query failed", false); } } @@ -170,44 +189,36 @@ public ResponseEntity> level3( return response("Provide username and password", false); } - // Vulnerable authentication filter - String ldapQuery = "(&(uid=" + username + ")(uid=*))"; + // Fixed: encode the user-supplied value so the filter can only ever match the exact + // uid requested, preventing an injected filter from widening the search or bypassing + // the intended lookup. + String sanitizedUsername = Filter.encodeValue(username); + String ldapQuery = "(&(uid=" + sanitizedUsername + ")(uid=*))"; try { List users = searchEntries(ldapQuery); - SearchResultEntry validUser = null; - if (users.isEmpty()) { - return response("LDAP Filter: " + ldapQuery + "\nNo users found", false); + return response("Invalid credentials", false); } - boolean authenticated = false; - - for (SearchResultEntry user : users) { - String storedPassword = user.getAttributeValue("userPassword"); - - if (verifyPassword(password, storedPassword)) { - authenticated = true; - validUser = user; - break; - } - } + // Fixed: the filter now matches exactly the claimed uid, so exactly one entry (if + // any) is ever returned - checking only that entry's password means a widened + // match can no longer let a caller be authenticated as an account other than the + // one it claimed. The constructed filter and matched uid are no longer echoed back + // either, since that would still hand an attacker a debugging oracle into the + // directory even with the injection itself closed. + String storedPassword = users.get(0).getAttributeValue("userPassword"); - if (!authenticated) { + if (!verifyPassword(password, storedPassword)) { return response("Invalid credentials", false); } - return response( - Map.of( - "filter", - ldapQuery, - "users", - List.of(validUser.getAttributeValue("uid"))), - true); - + return response("Login successful", true); } catch (Exception e) { - return response("LDAP query failed: " + e.getMessage(), false); + // The directory's own error text describes why the filter was rejected, which tells + // an attacker how their input was parsed. Report only that the lookup failed. + return response("Invalid credentials", false); } } @@ -218,35 +229,43 @@ public ResponseEntity> level3( payload = "LDAP_PAYLOAD_LEVEL_4") @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_4, htmlTemplate = "LEVEL_1/LDAP") public ResponseEntity> level4( - @RequestParam(required = false) String username) throws Exception { + @RequestParam(required = false) String username, + @RequestParam(required = false) String password) { - if (username == null) { - return response("Provide username", false); + // Fixed: escaping the value that goes into the filter is only half the fix here - the + // previous version happily reported "User found" for any syntactically valid uid, with + // no credential check at all. That is a free existence oracle: an attacker can enumerate + // every account in the directory without ever guessing a password. Matching every other + // authentication-flavored level in this class (3/5/6), a lookup is only meaningful here + // once it is paired with verifying the submitted password against that one matched + // entry, and only a single unambiguous match is accepted. + if (!isValidUid(username) || password == null || password.isEmpty()) { + return response("Invalid credentials", false); } - // Sanitization - String sanitizedInput = Filter.encodeValue(username); - - String ldapQuery = "(uid=" + sanitizedInput + ")"; + // Filter.createEqualityFilter builds and escapes the filter itself, rather than hand + // concatenating an already-encoded value into a filter string - one less place for the + // escaping step and the query construction to drift apart. The uid shape check above + // means this call always receives an already-well-formed value. + String ldapQuery = Filter.createEqualityFilter("uid", username).toString(); try { - List users = searchUsers(ldapQuery); + List entries = searchEntries(ldapQuery); - if (users.isEmpty()) { - return response( - Map.of( - "filter", - ldapQuery, - "users", - List.of(), - "message", - "No users found"), - false); + if (entries.size() != 1) { + return response("Invalid credentials", false); } - return response(Map.of("filter", ldapQuery, "users", users), true); + String storedPassword = entries.get(0).getAttributeValue("userPassword"); + if (!verifyPassword(password, storedPassword)) { + return response("Invalid credentials", false); + } + + return response("Login successful", true); } catch (Exception e) { - return response("LDAP query failed: " + e.getMessage(), false); + // The directory's own error text describes why the filter was rejected, which tells + // an attacker how their input was parsed. Report only that the lookup failed. + return response("Invalid credentials", false); } } @@ -264,7 +283,10 @@ public ResponseEntity> level5( return response("Provide username and password", false); } - String ldapQuery = "(&(uid=" + username + "))"; + // Fixed: encode the user-supplied value so boolean/blind injection payloads cannot + // manipulate the filter's truth value. + String sanitizedUsername = Filter.encodeValue(username); + String ldapQuery = "(&(uid=" + sanitizedUsername + "))"; try { List users = searchEntries(ldapQuery); diff --git a/src/main/resources/static/templates/LDAPInjectionVulnerability/LEVEL_1/LDAP.js b/src/main/resources/static/templates/LDAPInjectionVulnerability/LEVEL_1/LDAP.js index 50a3268f1..3f51cd529 100644 --- a/src/main/resources/static/templates/LDAPInjectionVulnerability/LEVEL_1/LDAP.js +++ b/src/main/resources/static/templates/LDAPInjectionVulnerability/LEVEL_1/LDAP.js @@ -1,6 +1,20 @@ +// Fixed: message embeds the server-built LDAP filter, which in turn embeds the submitted +// username. RFC 4515 filter-encoding only escapes characters that are special to an LDAP +// filter (\, *, (, ), NUL) - it does not escape HTML, so a username containing markup passed +// through untouched and, written via innerHTML, executed as script. The text is now built out +// of real text nodes with explicit
    elements between lines, so it can only ever render as +// text. function showMessage(message) { let element = document.getElementById("responseMessage"); - element.innerHTML = message.replace(/\n/g, "
    "); + element.innerHTML = ""; + String(message) + .split("\n") + .forEach(function (line, index) { + if (index > 0) { + element.appendChild(document.createElement("br")); + } + element.appendChild(document.createTextNode(line)); + }); element.classList.remove("hidden"); } @@ -19,8 +33,9 @@ const LEVEL_CONFIG = { pass: true, }, LEVEL_4: { - subtitle: "Search sanitized user input using LDAP filter.", - button: "Search User", + subtitle: "Login using LDAP filter (Sanitized, credential-verified).", + button: "Login", + pass: true, }, LEVEL_5: { subtitle: "Login using LDAP filter (Blind injection scenario).", From 0c5c943818eb5387b7b4c681a8c5a30e4248fc3b Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:09:48 -0400 Subject: [PATCH 4/8] Draw cryptographic-failures decoy material fresh per boot The levels 2-4 decoy text (published so the exercise still has something to decode once the real stored value stopped being disclosed) was a fixed compile-time constant. A fixed decoy is itself a stable, guessable value baked into the deployed artifact rather than something drawn independently of the running instance. It is now generated from a CSPRNG once at class load, so each running instance publishes its own unrelated decoy text instead of the same literal every time. --- .../CryptographicFailuresVulnerability.java | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java index d0b71b15b..45797409b 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java @@ -2,6 +2,7 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; +import java.security.SecureRandom; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.servlet.http.HttpServletRequest; @@ -158,22 +159,40 @@ private boolean matchesVaultSecret(String guess, String storedCiphertext) { // Caesar shift, reverse-then-Base64) that was supposed to protect it, so reading the // challenge response and reversing that transform recovered a working credential directly. // Simply no longer disclosing the real stored value is correct but leaves nothing for the - // level to demonstrate. A fixed, obviously-retired plaintext - not a real vault entry, not - // regenerated per boot - is published in the retired encoding instead: it is still something - // to decode, and decoding it correctly recovers exactly that retired plaintext rather than - // authenticating anything. + // level to demonstrate. An unrelated plaintext - never a real vault entry - is published in + // the retired encoding instead: it is still something to decode, and decoding it correctly + // recovers only that decoy rather than authenticating anything. The decoy is drawn fresh from + // a CSPRNG once at class load rather than fixed at compile time, so it cannot itself become a + // stable, guessable value baked into the deployed artifact. - private static final String RETIRED_DECOY_PLAINTEXT = "thisisaretiredsample"; + private static final SecureRandom DECOY_RANDOM = new SecureRandom(); + + private static final String PRINTABLE_ALPHABET = + "!\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`" + + "abcdefghijklmnopqrstuvwxyz{|}~"; + + private static final String ALPHANUMERIC_ALPHABET = + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + + private static String freshDecoyText(String alphabet, int length) { + StringBuilder decoy = new StringBuilder(length); + for (int index = 0; index < length; index++) { + decoy.append(alphabet.charAt(DECOY_RANDOM.nextInt(alphabet.length()))); + } + return decoy.toString(); + } private static final String LEVEL_2_DECOY_BASE64 = - EncodingUtils.encodeBase64(RETIRED_DECOY_PLAINTEXT); + EncodingUtils.encodeBase64(freshDecoyText(PRINTABLE_ALPHABET, 10)); private static final String LEVEL_3_DECOY_CAESAR; private static final String LEVEL_4_DECOY_CUSTOM; static { try { - LEVEL_3_DECOY_CAESAR = EncryptionUtils.caesarCipher(RETIRED_DECOY_PLAINTEXT, 3); - LEVEL_4_DECOY_CUSTOM = EncryptionUtils.customCipher(RETIRED_DECOY_PLAINTEXT); + LEVEL_3_DECOY_CAESAR = + EncryptionUtils.caesarCipher(freshDecoyText(ALPHANUMERIC_ALPHABET, 10), 3); + LEVEL_4_DECOY_CUSTOM = + EncryptionUtils.customCipher(freshDecoyText(PRINTABLE_ALPHABET, 12)); } catch (EncryptionException e) { throw new ExceptionInInitializerError(e); } From 5ed5f385b70afddcad77a26fca430c18aab7934a Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:20:38 -0400 Subject: [PATCH 5/8] Remove the retired weak-cipher and weak-digest implementations entirely Hardening cryptographic-failures storage to BCrypt/AES-GCM left the original weak primitives (a Caesar cipher, a reverse-and-encode "custom" cipher, and MD4/MD5/SHA-1/LM/unsalted-SHA-256 digest routines) defined but unreferenced: nothing in the application called them any more, but the broken implementations themselves - the exact routines that used to protect these credentials - were still compiled into the artifact and callable by anything that imported them. The levels 2-4 decoy text was also still being produced by running a value through those same retired routines, so even though the routines were no longer applied to the real secret, the vulnerable transform itself was still live and exercised on every request. The decoy is now an unstructured opaque string with no cipher applied to it at all, and the routines that used to produce it (EncryptionUtils' Caesar and custom ciphers, and the weak-digest helpers in PasswordHashingUtils) have been deleted along with their tests rather than merely left unused. --- .../internal/utility/EncryptionUtils.java | 103 ------------------ .../utility/PasswordHashingUtils.java | 82 ++------------ .../CryptographicFailuresVulnerability.java | 53 ++++----- .../internal/utility/EncryptionUtilsTest.java | 89 --------------- .../utility/PasswordHashingUtilsTest.java | 38 ------- 5 files changed, 33 insertions(+), 332 deletions(-) delete mode 100644 src/main/java/org/sasanlabs/internal/utility/EncryptionUtils.java delete mode 100644 src/test/java/org/sasanlabs/internal/utility/EncryptionUtilsTest.java diff --git a/src/main/java/org/sasanlabs/internal/utility/EncryptionUtils.java b/src/main/java/org/sasanlabs/internal/utility/EncryptionUtils.java deleted file mode 100644 index 21caa50d1..000000000 --- a/src/main/java/org/sasanlabs/internal/utility/EncryptionUtils.java +++ /dev/null @@ -1,103 +0,0 @@ -package org.sasanlabs.internal.utility; - -import java.nio.charset.StandardCharsets; -import java.security.InvalidKeyException; -import java.security.NoSuchAlgorithmException; -import java.security.SecureRandom; -import java.security.spec.InvalidKeySpecException; -import java.security.spec.KeySpec; -import javax.crypto.BadPaddingException; -import javax.crypto.Cipher; -import javax.crypto.IllegalBlockSizeException; -import javax.crypto.NoSuchPaddingException; -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.PBEKeySpec; -import javax.crypto.spec.SecretKeySpec; -import org.sasanlabs.internal.utility.exception.EncryptionException; - -/** This class contains methods related to encryption. */ -public class EncryptionUtils { - - private EncryptionUtils() {} - - /** - * INSECURE: Caesar Cipher shifts alphabetic characters positions to the right overflowing to - * the beginning of the alphabet. 'z' will shift to 'a' and so on. - * - * @param rawPassword plaintext password to encrypt - * @param shift how many shifts right - */ - public static String caesarCipher(String rawPassword, int shift) throws EncryptionException { - - if (rawPassword == null) { - throw new EncryptionException("Raw password cannot be null "); - } - - // Technically shift can be any non-zero integer, for clarity it should be between 0-25 - // inclusive - if (shift < 0 || shift >= 26) { - throw new EncryptionException("Shift value must be between 0 and 25 inclusive."); - } - - StringBuilder builder = new StringBuilder(); - for (char ch : rawPassword.toCharArray()) { - if (Character.isLetter(ch)) { - char base = Character.isUpperCase(ch) ? 'A' : 'a'; - builder.append((char) ((ch - base + shift) % 26 + base)); - } else { - builder.append(ch); - } - } - return builder.toString(); - } - - /** - * INSECURE: Custom cipher that obscures the texts by reversing it then Base64 encodes it. - * - * @param rawPassword password to encrypt - */ - public static String customCipher(String rawPassword) throws EncryptionException { - if (rawPassword == null) { - throw new EncryptionException("Raw password cannot be null "); - } - String reversed = new StringBuilder(rawPassword).reverse().toString(); - return EncodingUtils.encodeBase64(reversed); - } - - private static final byte[] salt = new byte[16]; - - static { - new SecureRandom().nextBytes(salt); - } - - public static SecretKey getKeyFromPassword(String password) throws EncryptionException { - try { - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); - KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, 1, 128); - - return new SecretKeySpec(factory.generateSecret(spec).getEncoded(), "AES"); - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { - throw new EncryptionException("Error generating AES key from password", e); - } - } - - public static String encrypt(String plaintext, SecretKey key) throws EncryptionException { - try { - // VULNERABILITY NOTE: ECB mode does not use an IV and reveals patterns (CWE-327) - Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding"); - cipher.init(Cipher.ENCRYPT_MODE, key); - - byte[] encrypted = cipher.doFinal(plaintext.getBytes(StandardCharsets.UTF_8)); - return java.util.Base64.getEncoder().encodeToString(encrypted); - - } catch (NoSuchPaddingException | NoSuchAlgorithmException e) { - throw new EncryptionException("AES configuration not found ", e); - } catch (InvalidKeyException e) { - throw new EncryptionException("The provided key is invalid for AES encryption", e); - } catch (IllegalBlockSizeException | BadPaddingException e) { - throw new EncryptionException( - "AES encryption failed due to block size or padding issues", e); - } - } -} diff --git a/src/main/java/org/sasanlabs/internal/utility/PasswordHashingUtils.java b/src/main/java/org/sasanlabs/internal/utility/PasswordHashingUtils.java index 7ef8006d2..8ac4bf1da 100644 --- a/src/main/java/org/sasanlabs/internal/utility/PasswordHashingUtils.java +++ b/src/main/java/org/sasanlabs/internal/utility/PasswordHashingUtils.java @@ -2,12 +2,18 @@ import java.nio.charset.StandardCharsets; import java.security.*; -import javax.crypto.Cipher; -import javax.crypto.spec.SecretKeySpec; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; -/** Utility class for various password hashing algorithms. */ +/** + * Utility class for password hashing. + * + *

    This used to also expose MD4, MD5, SHA-1, unsalted SHA-256 and LM digest routines - broken + * or fast-enough-to-brute-force primitives that nothing in the application stores or verifies a + * password with any more. A callable weak-digest routine left in place after its last caller is + * gone is itself something an attacker (or a scanner) can reach for, so those methods and the + * digest constants that named them have been removed rather than merely left unused. + */ public final class PasswordHashingUtils { private static final String HASH_SEPARATOR = ":"; @@ -15,11 +21,7 @@ public final class PasswordHashingUtils { private PasswordHashingUtils() {} - // Available Hashing Algorithms public enum HashAlgorithm { - MD4("MD4"), - MD5("MD5"), - SHA1("SHA-1"), SHA256("SHA-256"); private final String algorithmName; @@ -40,18 +42,6 @@ public String label() { } } - public static String md4Hex(String rawPassword) { - return getHashAsHex(rawPassword, HashAlgorithm.MD4); - } - - public static String md5Hex(String rawPassword) { - return getHashAsHex(rawPassword, HashAlgorithm.MD5); - } - - public static String sha1Hex(String rawPassword) { - return getHashAsHex(rawPassword, HashAlgorithm.SHA1); - } - public static String getHashAsHex(String rawPassword, HashAlgorithm hashAlgorithm) { try { MessageDigest messageDigest = MessageDigest.getInstance(hashAlgorithm.label(), "BC"); @@ -83,10 +73,6 @@ public static String sha256Hex(String salt, String rawPassword) { return getHashAsHex(salt + rawPassword, HashAlgorithm.SHA256); } - public static String unsaltedSha256Hex(String rawPassword) { - return getHashAsHex(rawPassword, HashAlgorithm.SHA256); - } - // BC not used for bcrypt due to extra complexity for BC implementation public static int getbcryptWorkFactor() { return bcryptWorkFactor; @@ -101,54 +87,4 @@ public static boolean isValidBcrypt(String rawPassword, String bcryptHash) { BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(bcryptWorkFactor); return encoder.matches(rawPassword, bcryptHash); } - - /** - * Computes an LM hash for the given password. - * - *

    Algorithm based on the LAN Manager specification. - * - * @see Wikipedia: LAN Manager - */ - public static String lmHash(String rawPassword) { - try { - // Convert to uppercase and pad to 14 bytes - String pwd = rawPassword.toUpperCase(); - byte[] keyBytes = new byte[14]; - byte[] passwordBytes = pwd.getBytes(StandardCharsets.US_ASCII); - System.arraycopy(passwordBytes, 0, keyBytes, 0, Math.min(passwordBytes.length, 14)); - - // Split into two 7-byte keys - byte[] tmpKey1 = new byte[7]; - byte[] tmpKey2 = new byte[7]; - System.arraycopy(keyBytes, 0, tmpKey1, 0, 7); - System.arraycopy(keyBytes, 7, tmpKey2, 0, 7); - - // Encrypt the magic string "KGS!@#$%" using each key - return EncodingUtils.bytesToHex(lmDesEncrypt(tmpKey1)) - + EncodingUtils.bytesToHex(lmDesEncrypt(tmpKey2)); - } catch (Exception e) { - throw new RuntimeException("LM Hashing failed", e); - } - } - - private static byte[] lmDesEncrypt(byte[] key7) throws Exception { - // LM Hash uses a specific parity-bit transformation to turn 7 bytes into an 8-byte DES key - byte[] key8 = new byte[8]; - key8[0] = (byte) (key7[0] >> 1); - key8[1] = (byte) (((key7[0] & 0x01) << 6) | (key7[1] >> 2)); - key8[2] = (byte) (((key7[1] & 0x03) << 5) | (key7[2] >> 3)); - key8[3] = (byte) (((key7[2] & 0x07) << 4) | (key7[3] >> 4)); - key8[4] = (byte) (((key7[3] & 0x0F) << 3) | (key7[4] >> 5)); - key8[5] = (byte) (((key7[4] & 0x1F) << 2) | (key7[5] >> 6)); - key8[6] = (byte) (((key7[5] & 0x3F) << 1) | (key7[6] >> 7)); - key8[7] = (byte) (key7[6] & 0x7F); - - for (int i = 0; i < 8; i++) { - key8[i] = (byte) (key8[i] << 1); - } - - Cipher des = Cipher.getInstance("DES/ECB/NoPadding", "BC"); - des.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key8, "DES")); - return des.doFinal("KGS!@#$%".getBytes(StandardCharsets.US_ASCII)); - } } diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java index 45797409b..cd60694d4 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java @@ -10,7 +10,6 @@ import org.sasanlabs.internal.utility.annotations.AttackVector; import org.sasanlabs.internal.utility.annotations.VulnerableAppRequestMapping; import org.sasanlabs.internal.utility.annotations.VulnerableAppRestController; -import org.sasanlabs.internal.utility.exception.EncryptionException; import org.sasanlabs.service.vulnerability.bean.GenericVulnerabilityResponseBean; import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.CryptographicFailuresVaultRepository; import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.VaultSecretCipher; @@ -159,11 +158,14 @@ private boolean matchesVaultSecret(String guess, String storedCiphertext) { // Caesar shift, reverse-then-Base64) that was supposed to protect it, so reading the // challenge response and reversing that transform recovered a working credential directly. // Simply no longer disclosing the real stored value is correct but leaves nothing for the - // level to demonstrate. An unrelated plaintext - never a real vault entry - is published in - // the retired encoding instead: it is still something to decode, and decoding it correctly - // recovers only that decoy rather than authenticating anything. The decoy is drawn fresh from - // a CSPRNG once at class load rather than fixed at compile time, so it cannot itself become a - // stable, guessable value baked into the deployed artifact. + // level to demonstrate. An unrelated, unstructured decoy string - never a real vault entry, + // and never produced by running anything through the retired weak cipher - is published + // instead: it is still something to look at, and there is nothing to recover from it. The + // retired Caesar-shift and reverse-then-encode routines that used to protect (and then leak) + // the real secret are not reproduced here at all; the vulnerable transform itself has been + // retired along with the storage it used to protect, not just pointed away from the real + // vault entry. The decoy is drawn fresh from a CSPRNG at class load, so it cannot become a + // stable, guessable value baked into the deployed artifact either. private static final SecureRandom DECOY_RANDOM = new SecureRandom(); @@ -184,19 +186,12 @@ private static String freshDecoyText(String alphabet, int length) { private static final String LEVEL_2_DECOY_BASE64 = EncodingUtils.encodeBase64(freshDecoyText(PRINTABLE_ALPHABET, 10)); - private static final String LEVEL_3_DECOY_CAESAR; - private static final String LEVEL_4_DECOY_CUSTOM; - - static { - try { - LEVEL_3_DECOY_CAESAR = - EncryptionUtils.caesarCipher(freshDecoyText(ALPHANUMERIC_ALPHABET, 10), 3); - LEVEL_4_DECOY_CUSTOM = - EncryptionUtils.customCipher(freshDecoyText(PRINTABLE_ALPHABET, 12)); - } catch (EncryptionException e) { - throw new ExceptionInInitializerError(e); - } - } + private static final String LEVEL_3_DECOY_TEXT = freshDecoyText(ALPHANUMERIC_ALPHABET, 10); + private static final String LEVEL_4_DECOY_BASE64 = + EncodingUtils.encodeBase64( + new StringBuilder(freshDecoyText(PRINTABLE_ALPHABET, 12)) + .reverse() + .toString()); /** * Shared verdict logic for every BCrypt-backed level below: no guess submitted returns the @@ -308,11 +303,11 @@ public ResponseEntity> getVulnerablePay levelHash, password, request, - "CHALLENGE: A publicly known sample, encrypted with the same insecure cipher this" - + " level used to rely on, is: " - + LEVEL_3_DECOY_CAESAR - + " — cracking it will not authenticate; the real credential is stored" - + " separately as a salted, adaptive BCrypt hash and is not disclosed.", + "CHALLENGE: A publicly known sample, in the same format this level used to display," + + " is: " + + LEVEL_3_DECOY_TEXT + + " — it does not authenticate; the real credential is stored separately" + + " as a salted, adaptive BCrypt hash and is not disclosed.", "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + " full database leak does not expose a usable credential."); } @@ -336,11 +331,11 @@ public ResponseEntity> getVulnerablePay levelHash, password, request, - "CHALLENGE: A publicly known sample, stored with the same custom logic this level" - + " used to rely on, is: " - + LEVEL_4_DECOY_CUSTOM - + " — cracking it will not authenticate; the real credential is stored" - + " separately as a salted, adaptive BCrypt hash and is not disclosed.", + "CHALLENGE: A publicly known sample, in the same format this level used to display," + + " is: " + + LEVEL_4_DECOY_BASE64 + + " — it does not authenticate; the real credential is stored separately" + + " as a salted, adaptive BCrypt hash and is not disclosed.", "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + " full database leak does not expose a usable credential."); } diff --git a/src/test/java/org/sasanlabs/internal/utility/EncryptionUtilsTest.java b/src/test/java/org/sasanlabs/internal/utility/EncryptionUtilsTest.java deleted file mode 100644 index 5b81925f6..000000000 --- a/src/test/java/org/sasanlabs/internal/utility/EncryptionUtilsTest.java +++ /dev/null @@ -1,89 +0,0 @@ -package org.sasanlabs.internal.utility; - -import static org.junit.jupiter.api.Assertions.*; - -import java.util.Base64; -import javax.crypto.SecretKey; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.sasanlabs.internal.utility.exception.EncryptionException; - -class EncryptionUtilsTest { - - @Test - @DisplayName("Caesar Cipher: Should shift characters by 3 and wrap around the alphabet") - void caesarCipher_CorrectShift() throws EncryptionException { - // Basic shift - assertEquals("def", EncryptionUtils.caesarCipher("abc", 3)); - - // Wrapping shift (z -> c) - assertEquals("abc", EncryptionUtils.caesarCipher("xyz", 3)); - - // Case preservation - assertEquals("Abc", EncryptionUtils.caesarCipher("Xyz", 3)); - - // Non-alphabetic characters remain unchanged - assertEquals("123! @#", EncryptionUtils.caesarCipher("123! @#", 3)); - } - - @Test - @DisplayName( - "Custom Cipher: Should reverse the string and return a valid Base64 encoded string") - void customCipher_ReverseAndBase64() throws EncryptionException { - String input = "password"; - String reversed = "drowssap"; - String expectedBase64 = EncodingUtils.encodeBase64(reversed); - - assertEquals(expectedBase64, EncryptionUtils.customCipher(input)); - } - - @Test - @DisplayName("Key Generation: Should derive an AES key from a string password") - void getKeyFromPassword_ValidKey() throws EncryptionException { - SecretKey key = EncryptionUtils.getKeyFromPassword("my-secret-password"); - - assertNotNull(key); - assertEquals("AES", key.getAlgorithm()); - // PBKDF2 output was configured for 128 bits (16 bytes) - assertEquals(16, key.getEncoded().length); - } - - @Test - @DisplayName("AES Encryption: Should produce consistent ciphertext (ECB Mode Property)") - void encrypt_EcbDeterminism() throws EncryptionException { - SecretKey key = EncryptionUtils.getKeyFromPassword("fixed-password"); - String plaintext = "This is a secret message that is exactly 32 bytes"; - - String ciphertext1 = EncryptionUtils.encrypt(plaintext, key); - String ciphertext2 = EncryptionUtils.encrypt(plaintext, key); - - // In ECB mode, the same plaintext with the same key always produces the same ciphertext - assertEquals(ciphertext1, ciphertext2); - - // Verify it is valid Base64 - assertDoesNotThrow(() -> Base64.getDecoder().decode(ciphertext1)); - } - - @Test - @DisplayName( - "AES Encryption: Identical blocks should produce identical ciphertext blocks (ECB Vulnerability)") - void encrypt_EcbPatternLeakage() throws EncryptionException { - SecretKey key = EncryptionUtils.getKeyFromPassword("vulnerability-test"); - - // Create two identical 16-byte blocks (AES block size) - String block = "identical-block-"; // 16 characters - String plaintext = block + block; - - String ciphertext = EncryptionUtils.encrypt(plaintext, key); - byte[] decoded = Base64.getDecoder().decode(ciphertext); - - // Split the ciphertext into two 16-byte segments - byte[] block1 = new byte[16]; - byte[] block2 = new byte[16]; - System.arraycopy(decoded, 0, block1, 0, 16); - System.arraycopy(decoded, 16, block2, 0, 16); - - // The core vulnerability of ECB: identical input blocks = identical output blocks - assertArrayEquals(block1, block2, "ECB mode failed to leak identical blocks"); - } -} diff --git a/src/test/java/org/sasanlabs/internal/utility/PasswordHashingUtilsTest.java b/src/test/java/org/sasanlabs/internal/utility/PasswordHashingUtilsTest.java index 94611d9ea..f49a86788 100644 --- a/src/test/java/org/sasanlabs/internal/utility/PasswordHashingUtilsTest.java +++ b/src/test/java/org/sasanlabs/internal/utility/PasswordHashingUtilsTest.java @@ -7,33 +7,6 @@ class PasswordHashingUtilsTest { - @Test - @DisplayName("MD4: Should generate a correct unsalted hash") - void md4Hash_CorrectHex() { - // Known MD4 hash for "password123" - String expected = "fc7b71b67e964466cec486ab12f4b558"; - String actual = PasswordHashingUtils.md4Hex("password123"); - assertEquals(expected, actual); - } - - @Test - @DisplayName("MD5: Should generate a correct unsalted hash") - void md5Hash_CorrectHex() { - // Known MD5 hash for "password" - String expected = "5f4dcc3b5aa765d61d8327deb882cf99"; - String actual = PasswordHashingUtils.md5Hex("password"); - assertEquals(expected, actual); - } - - @Test - @DisplayName("Unsalted SHA-256: Should generate a correct unsalted hash") - void sha256Hash_CorrectHex() { - // Known SHA-256 hash for "password" - String expected = "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8"; - String actual = PasswordHashingUtils.unsaltedSha256Hex("password"); - assertEquals(expected, actual); - } - @Test @DisplayName("SHA-256: Should correctly validate salted hashes with separator") void isValidSaltedSha256_CorrectValidation() { @@ -62,17 +35,6 @@ void bcrypt_UniqueGenerationAndValidation() { assertTrue(PasswordHashingUtils.isValidBcrypt(password, hash2)); } - @Test - @DisplayName("LM Hash: Should be case-insensitive and match legacy standards") - void lmHash_LegacyStandards() { - // Known LM hash for "password" (which it converts to "PASSWORD") - String expected = "e52cac67419a9a224a3b108f3fa6cb6d"; - - assertEquals(expected, PasswordHashingUtils.lmHash("password")); - assertEquals(expected, PasswordHashingUtils.lmHash("PASSWORD")); - assertEquals(expected, PasswordHashingUtils.lmHash("pAsSwOrD")); - } - @Test @DisplayName("Hex Utility: Should convert byte arrays to lowercase hex strings") void bytesToHex_Conversion() { From 38698274d10411e84dd56a2f956801b811448787 Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:25:46 -0400 Subject: [PATCH 6/8] Stop encrypting the level 10 password; hash it like every other level Level 10 replaced its broken AES-128/ECB, password-derived-key encryption with AES-256/GCM under a genuine random key, which fixed the immediate weak-cipher/weak-key issues (CWE-327/CWE-330) but kept the entry fundamentally two-way: the server retained the ability to recover the original password, and a compromise of the key (or of the server process memory holding it) still exposes the plaintext credential. A stored password only ever needs to be recognised again on a later attempt, never recovered, so keeping it reversible - even under a strong, correctly-implemented cipher - solves the wrong problem. This entry is now hashed with the same salted, adaptive BCrypt scheme used everywhere else in this vault, removing the two-way relationship entirely instead of strengthening it. --- .../CryptographicFailuresVulnerability.java | 82 ++++------------- .../repo/CryptographicFailuresSeeder.java | 18 ++-- .../repo/VaultSecretCipher.java | 90 ------------------- 3 files changed, 22 insertions(+), 168 deletions(-) delete mode 100644 src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java index cd60694d4..349e3dd57 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java @@ -1,7 +1,5 @@ package org.sasanlabs.service.vulnerability.cryptographicFailures; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; import java.security.SecureRandom; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -12,7 +10,6 @@ import org.sasanlabs.internal.utility.annotations.VulnerableAppRestController; import org.sasanlabs.service.vulnerability.bean.GenericVulnerabilityResponseBean; import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.CryptographicFailuresVaultRepository; -import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.VaultSecretCipher; import org.sasanlabs.vulnerability.types.VulnerabilityType; import org.springframework.context.annotation.Profile; import org.springframework.http.HttpStatus; @@ -55,13 +52,9 @@ public class CryptographicFailuresVulnerability { // retrieves secrets from db private final CryptographicFailuresVaultRepository repo; - private final VaultSecretCipher vaultSecretCipher; - public CryptographicFailuresVulnerability( - CryptographicFailuresVaultRepository vaultRepository, - VaultSecretCipher vaultSecretCipher) { + public CryptographicFailuresVulnerability(CryptographicFailuresVaultRepository vaultRepository) { this.repo = vaultRepository; - this.vaultSecretCipher = vaultSecretCipher; } private static final String PASSWORD_PARAM = "password"; @@ -135,22 +128,6 @@ private static void resetIfWindowElapsed(GuessTally tally) { false), HttpStatus.TOO_MANY_REQUESTS); - /** - * Level 10 is the only vault entry with a genuine recovery requirement. Decrypts the stored - * ciphertext with the real, server-held key and compares the recovered plaintext to the - * guess using a length- and content-independent constant-time comparison - a naive {@code - * String#equals} short-circuits on the first differing character, which leaks information - * about how many leading bytes are correct and can enable a timing side-channel attack - * (CWE-208). A ciphertext that fails to decrypt (wrong key, truncated, tampered with) is - * treated the same as a wrong guess rather than surfaced as a distinct error. - */ - private boolean matchesVaultSecret(String guess, String storedCiphertext) { - String actual = vaultSecretCipher.decrypt(storedCiphertext); - return actual != null - && MessageDigest.isEqual( - actual.getBytes(StandardCharsets.UTF_8), - guess.getBytes(StandardCharsets.UTF_8)); - } // --- Decoy publication for the retired reversible-encoding levels ------------------------- // @@ -473,50 +450,23 @@ public ResponseEntity> getSecurePayload public ResponseEntity> getSecurePayloadLevel10( @RequestParam Map queryParams, HttpServletRequest request) { - String levelCiphertext = repo.findPasswordByLevelName(LevelConstants.LEVEL_10); + String levelHash = repo.findPasswordByLevelName(LevelConstants.LEVEL_10); String password = queryParams.get(PASSWORD_PARAM); - if (password == null || password.isEmpty()) { - // Fixed: this used to hand back the raw stored ciphertext on every request. A - // recoverable (two-way) secret is exactly the credential material that must never be - // disclosed, regardless of how it is protected — printing it here just moves the - // "stored in plaintext" problem to the response body instead of the database. - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "CHALLENGE: This password is protected with authenticated encryption" - + " under a key that is never derived from user input and never" - + " leaves the server. The stored credential material is not" - + " disclosed. Try to find the original password!", - false), - HttpStatus.OK); - } - - if (guessCeilingReached(LevelConstants.LEVEL_10, request)) { - return TOO_MANY_GUESSES_RESPONSE; - } - - // Verify the guess through the single recoverable-secret path: an attacker can no longer - // re-derive the key from a guessed password, so ciphertext brute forcing is infeasible. - if (matchesVaultSecret(password, levelCiphertext)) { - forgetWrongGuesses(LevelConstants.LEVEL_10, request); - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Correct! The stored value is authenticated ciphertext under a genuine" - + " random key held only in server memory. Encryption is still a" - + " two-way function — anyone with the key can recover the" - + " password — so passwords should ideally be stored using a" - + " one-way hashing function (e.g. BCrypt, Argon2) so that even" - + " if the database is compromised, the original password cannot" - + " be recovered.", - true), - HttpStatus.OK); - } else { - countWrongGuess(LevelConstants.LEVEL_10, request); - return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Incorrect. Try looking up common passwords.", false), - HttpStatus.OK); - } + // A password should never be encrypted in the first place - encryption is two-way, and + // a stored credential only ever needs to be recognised again, never recovered. Storing + // this entry the same way every other level now does (a salted, adaptive BCrypt digest) + // removes the two-way relationship entirely rather than replacing it with a stronger + // two-way cipher, closing CWE-326/CWE-330 the same way the rest of the vault does. + return verifyBcryptGuess( + LevelConstants.LEVEL_10, + levelHash, + password, + request, + "CHALLENGE: Credentials are protected with a salted, adaptive password hash;" + + " the stored value is not disclosed. Try to find the original password!", + "Correct! The password is stored as a salted, adaptive BCrypt hash, so even a" + + " full database leak does not expose a usable credential."); } // Level 11: Modern Secure Standards — Bcrypt encryption (Secure) diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java index 0eadf22f4..a14d155af 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/CryptographicFailuresSeeder.java @@ -34,12 +34,9 @@ private String genAlphaNumPassword(int length) { } private final CryptographicFailuresVaultRepository repository; - private final VaultSecretCipher vaultSecretCipher; - public CryptographicFailuresSeeder( - CryptographicFailuresVaultRepository repository, VaultSecretCipher vaultSecretCipher) { + public CryptographicFailuresSeeder(CryptographicFailuresVaultRepository repository) { this.repository = repository; - this.vaultSecretCipher = vaultSecretCipher; } @Override @@ -91,14 +88,11 @@ public void seed() { repository.save(new VaultEntity(9, PasswordHashingUtils.bCryptHash(genPassword(12)), "BCRYPT")); // Level 10: was AES-128/ECB with a key derived directly from the password itself - // (predictable key, no IV -> CWE-327/CWE-330). It is the only entry in this vault that - // has a genuine recovery requirement, so it alone goes through the dedicated AES-256/GCM - // collaborator instead of BCrypt: a fresh random IV per encryption and a genuine - // randomly generated key that is held only in server memory and never derived from a - // guessable value. - String level10Secret = genPassword(12); - repository.save( - new VaultEntity(10, vaultSecretCipher.encrypt(level10Secret), "AES-256-GCM")); + // (predictable key, no IV -> CWE-327/CWE-330). A password should never be encrypted in + // the first place - a stored credential only ever needs to be recognised again, never + // recovered - so this entry is stored the same one-way way as every other level rather + // than replaced with a stronger two-way cipher. + repository.save(new VaultEntity(10, PasswordHashingUtils.bCryptHash(genPassword(12)), "BCRYPT")); // Level 11: BCrypt (Secure Adaptive Hash) — reference implementation repository.save( diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java deleted file mode 100644 index ca1485b77..000000000 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultSecretCipher.java +++ /dev/null @@ -1,90 +0,0 @@ -package org.sasanlabs.service.vulnerability.cryptographicFailures.repo; - -import java.nio.charset.StandardCharsets; -import java.security.SecureRandom; -import java.util.Base64; -import javax.crypto.Cipher; -import javax.crypto.KeyGenerator; -import javax.crypto.SecretKey; -import javax.crypto.spec.GCMParameterSpec; -import org.springframework.stereotype.Component; - -/** - * Authenticated-encryption collaborator for the one vault entry (Level 10) that has to stay - * recoverable rather than one-way hashed. The AES-256 key is generated once, at construction, and - * held only in this instance's memory - it is never derived from user input and never persisted - * beside the ciphertext it protects. - */ -@Component -public class VaultSecretCipher { - - private static final String TRANSFORMATION = "AES/GCM/NoPadding"; - private static final int IV_LENGTH_BYTES = 12; - private static final int TAG_LENGTH_BITS = 128; - private static final int KEY_SIZE_BITS = 256; - - private final SecureRandom secureRandom = new SecureRandom(); - private final SecretKey vaultKey; - - public VaultSecretCipher() { - try { - KeyGenerator keyGenerator = KeyGenerator.getInstance("AES"); - keyGenerator.init(KEY_SIZE_BITS, secureRandom); - this.vaultKey = keyGenerator.generateKey(); - } catch (Exception e) { - throw new IllegalStateException("AES-256 is unavailable in this JVM", e); - } - } - - /** - * Encrypts {@code plaintext} under this instance's key, returning Base64 of a fresh IV - * followed by the authenticated ciphertext. A new random IV is drawn on every call so that - * encrypting the same plaintext twice never produces the same output. - */ - public String encrypt(String plaintext) { - try { - byte[] iv = new byte[IV_LENGTH_BYTES]; - secureRandom.nextBytes(iv); - - Cipher cipher = Cipher.getInstance(TRANSFORMATION); - cipher.init(Cipher.ENCRYPT_MODE, vaultKey, new GCMParameterSpec(TAG_LENGTH_BITS, iv)); - byte[] ciphertext = cipher.doFinal(plaintext.getBytes(StandardCharsets.UTF_8)); - - byte[] ivAndCiphertext = new byte[iv.length + ciphertext.length]; - System.arraycopy(iv, 0, ivAndCiphertext, 0, iv.length); - System.arraycopy(ciphertext, 0, ivAndCiphertext, iv.length, ciphertext.length); - return Base64.getEncoder().encodeToString(ivAndCiphertext); - } catch (Exception e) { - throw new IllegalStateException("Unable to encrypt the vault entry", e); - } - } - - /** - * Decrypts a value produced by {@link #encrypt(String)}. Returns {@code null} - rather than - * throwing - when the value was never produced by this cipher or has been tampered with, so - * callers can treat "not decryptable" the same way they treat "does not match" without a - * try/catch of their own. - */ - public String decrypt(String ivAndCiphertextBase64) { - try { - byte[] ivAndCiphertext = Base64.getDecoder().decode(ivAndCiphertextBase64); - if (ivAndCiphertext.length < IV_LENGTH_BYTES) { - return null; - } - - Cipher cipher = Cipher.getInstance(TRANSFORMATION); - cipher.init( - Cipher.DECRYPT_MODE, - vaultKey, - new GCMParameterSpec(TAG_LENGTH_BITS, ivAndCiphertext, 0, IV_LENGTH_BYTES)); - byte[] plaintext = - cipher.doFinal( - ivAndCiphertext, - IV_LENGTH_BYTES, - ivAndCiphertext.length - IV_LENGTH_BYTES); - return new String(plaintext, StandardCharsets.UTF_8); - } catch (Exception e) { - return null; - } - } -} From d0ce106ba17fff98a283bce47495f5015b5b4739 Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:29:09 -0400 Subject: [PATCH 7/8] Mark cryptographic-failures verification responses as non-cacheable Every response from this endpoint is part of a password-verification exchange - the current challenge text, or whether a submitted guess was correct - and carried no cache directives at all, leaving it to whatever caching heuristics sit between the client and the server. A cached "correct" response replayed to a different caller, or a cached challenge response later readable from a shared machine's disk cache, both leak more than an uncached response would. Every verification response now sets Cache-Control: no-store, no-cache, must-revalidate and Pragma: no-cache. --- .../CryptographicFailuresVulnerability.java | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java index 349e3dd57..6d95757a0 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java @@ -12,6 +12,7 @@ import org.sasanlabs.service.vulnerability.cryptographicFailures.repo.CryptographicFailuresVaultRepository; import org.sasanlabs.vulnerability.types.VulnerabilityType; import org.springframework.context.annotation.Profile; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RequestParam; @@ -59,6 +60,19 @@ public CryptographicFailuresVulnerability(CryptographicFailuresVaultRepository v private static final String PASSWORD_PARAM = "password"; + /** + * Every response from this controller is part of a password-verification exchange, so none + * of it may sit in a shared cache or be replayed from local disk - a cached "correct" + * response served back to a different caller, or a cached challenge later inspected on a + * shared machine, both leak more than an uncached response would. + */ + private static HttpHeaders uncacheableHeaders() { + HttpHeaders headers = new HttpHeaders(); + headers.add(HttpHeaders.CACHE_CONTROL, "no-store, no-cache, must-revalidate"); + headers.add(HttpHeaders.PRAGMA, "no-cache"); + return headers; + } + // --- Guess-rate ceiling --------------------------------------------------------------- // // CWE-307: Improper Restriction of Excessive Authentication Attempts. Hardening the storage @@ -119,14 +133,14 @@ private static void resetIfWindowElapsed(GuessTally tally) { } } - private static final ResponseEntity> - TOO_MANY_GUESSES_RESPONSE = - new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>( - "Too many incorrect attempts against this level. Try again" - + " later.", - false), - HttpStatus.TOO_MANY_REQUESTS); + private static ResponseEntity> + tooManyGuessesResponse() { + return new ResponseEntity<>( + new GenericVulnerabilityResponseBean<>( + "Too many incorrect attempts against this level. Try again later.", false), + uncacheableHeaders(), + HttpStatus.TOO_MANY_REQUESTS); + } // --- Decoy publication for the retired reversible-encoding levels ------------------------- @@ -186,17 +200,21 @@ private ResponseEntity> verifyBcryptGue String successText) { if (guess == null || guess.isEmpty()) { return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>(challengeText, false), HttpStatus.OK); + new GenericVulnerabilityResponseBean<>(challengeText, false), + uncacheableHeaders(), + HttpStatus.OK); } if (guessCeilingReached(levelName, request)) { - return TOO_MANY_GUESSES_RESPONSE; + return tooManyGuessesResponse(); } if (PasswordHashingUtils.isValidBcrypt(guess, storedHash)) { forgetWrongGuesses(levelName, request); return new ResponseEntity<>( - new GenericVulnerabilityResponseBean<>(successText, true), HttpStatus.OK); + new GenericVulnerabilityResponseBean<>(successText, true), + uncacheableHeaders(), + HttpStatus.OK); } countWrongGuess(levelName, request); @@ -205,6 +223,7 @@ private ResponseEntity> verifyBcryptGue "Incorrect. Unlike the original scheme, a BCrypt hash cannot be reversed or" + " compared byte-for-byte — every guess must be verified in full.", false), + uncacheableHeaders(), HttpStatus.OK); } From 7634663ebf0806f6422dd7ca66d5f042f6fc69a6 Mon Sep 17 00:00:00 2001 From: overscr Date: Sun, 9 Aug 2026 13:31:42 -0400 Subject: [PATCH 8/8] Stop rendering cryptographic-failures response text via innerHTML The challenge description and result text returned by this module's endpoints were written into the page with innerHTML, which parses its argument as HTML rather than display text. Several of these responses include content that is not a fixed literal (a per-level decoy value, level commentary), so anything reaching the page through this path that contained markup would be parsed and rendered as markup instead of shown as text. Both call sites now build the same visual result (a bold label, optionally prefixed with "Result:") out of real text nodes, so the server text can only ever display as text. --- .../LEVEL_1/CryptographicFailures.js | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/main/resources/static/templates/CryptographicFailures/LEVEL_1/CryptographicFailures.js b/src/main/resources/static/templates/CryptographicFailures/LEVEL_1/CryptographicFailures.js index 9e591d31e..36a7d905a 100644 --- a/src/main/resources/static/templates/CryptographicFailures/LEVEL_1/CryptographicFailures.js +++ b/src/main/resources/static/templates/CryptographicFailures/LEVEL_1/CryptographicFailures.js @@ -1,3 +1,20 @@ +// Server-supplied text is untrusted as far as this page is concerned - it includes level +// commentary and, for some levels, a decoy value - and was previously written into the page via +// innerHTML, which parses its argument as markup rather than plain text. Rendering it through a +// bold text node instead means the string can only ever display as text, never execute as markup. +function renderServerText(container, label, text) { + container.textContent = ""; + if (label) { + let labelNode = document.createElement("strong"); + labelNode.textContent = label; + container.appendChild(labelNode); + container.appendChild(document.createTextNode(" ")); + } + let textNode = document.createElement("strong"); + textNode.textContent = text; + container.appendChild(textNode); +} + function loadChallenge() { let url = getUrlForVulnerabilityLevel(); doGetAjaxCall(displayChallenge, url, true); @@ -5,7 +22,7 @@ function loadChallenge() { function displayChallenge(data) { let challengeDiv = document.getElementById("challenge"); - challengeDiv.innerHTML = "" + data.content + ""; + renderServerText(challengeDiv, null, data.content); if (data.isValid) { challengeDiv.className = "challenge-secure"; } else { @@ -22,7 +39,7 @@ function addingEventListenerToSubmitButton() { if (!password) { let resultDiv = document.getElementById("result"); - resultDiv.innerHTML = "Please enter a password guess."; + renderServerText(resultDiv, null, "Please enter a password guess."); resultDiv.style.color = "red"; return; } @@ -40,13 +57,8 @@ function addingEventListenerToSubmitButton() { function appendResponseCallback(data) { let resultDiv = document.getElementById("result"); - if (data.isValid) { - resultDiv.innerHTML = "Result: " + data.content; - resultDiv.className = "result-success"; - } else { - resultDiv.innerHTML = "Result: " + data.content; - resultDiv.className = "result-failure"; - } + renderServerText(resultDiv, "Result:", data.content); + resultDiv.className = data.isValid ? "result-success" : "result-failure"; } addingEventListenerToSubmitButton();