From 64e0eb55abafdd66b5b64148c43b4a5e736de7a6 Mon Sep 17 00:00:00 2001
From: Attalla <37474787+attalla1@users.noreply.github.com>
Date: Sun, 9 Aug 2026 16:35:04 +0000
Subject: [PATCH] fix(VulnerableApp): apply latest security remediations
(squashed off dc34-ctf)
---
.../VulnerableAppConfiguration.java | 80 +-
.../internal/utility/EncodingUtils.java | 9 +-
.../internal/utility/EncryptionUtils.java | 103 ---
.../utility/PasswordHashingUtils.java | 78 +-
.../service/email/EmailServiceImpl.java | 40 +-
.../authentication/AuthLoginService.java | 245 ++++--
.../AuthenticationVulnerability.java | 41 +-
.../CachePoisoningVulnerability.java | 121 ++-
.../ClickjackingVulnerability.java | 32 +-
.../commandInjection/CommandInjection.java | 66 +-
.../CryptographicFailuresVulnerability.java | 574 +++++--------
.../repo/CryptographicFailuresSeeder.java | 100 +--
.../repo/VaultCipher.java | 72 ++
.../fileupload/PreflightController.java | 65 +-
.../fileupload/UnrestrictedFileUpload.java | 245 ++++--
.../vulnerability/idor/IDORVulnerability.java | 116 ++-
.../vulnerability/jwt/JWTVulnerability.java | 790 ++++++++----------
.../vulnerability/jwt/bean/JWTUtils.java | 10 +-
.../vulnerability/jwt/impl/JWTValidator.java | 367 ++++----
.../jwt/keys/JWTAlgorithmKMS.java | 108 ++-
.../jwt/keys/SymmetricAlgorithmKey.java | 5 +-
.../LDAPInjectionVulnerability.java | 143 ++--
.../Http3xxStatusCodeBasedInjection.java | 185 ++--
.../passwordReset/PasswordResetService.java | 172 ++--
.../PathTraversalVulnerability.java | 210 ++---
.../vulnerability/rfi/UrlParamBasedRFI.java | 192 ++++-
.../SessionManagementService.java | 178 ++--
.../SessionManagementVulnerability.java | 24 +-
.../BlindSQLInjectionVulnerability.java | 64 +-
.../ErrorBasedSQLInjectionVulnerability.java | 195 ++---
.../UnionBasedSQLInjectionVulnerability.java | 36 +-
.../vulnerability/ssrf/SSRFVulnerability.java | 231 ++++-
.../PersistentXSSInHTMLTagVulnerability.java | 96 +--
.../xss/reflected/XSSInImgTagAttribute.java | 108 ++-
.../reflected/XSSWithHtmlTagInjection.java | 39 +-
.../vulnerability/xxe/XXEVulnerability.java | 54 +-
src/main/resources/application.properties | 7 +
.../scripts/Authentication/db/data.sql | 47 +-
.../JWT}/keys/private_key.pem | 0
.../JWT}/keys/public_crt.pem | 0
.../static/password-reset/reset.html | 15 +-
.../CommandInjection/LEVEL_1/CI_Level1.js | 5 +-
.../JWTVulnerability/LEVEL_1/JWT_Level1.js | 19 +-
.../LEVEL_1/LDAP.js | 19 +-
.../SSRFVulnerability/LEVEL_1/SSRF.js | 53 +-
.../LEVEL_1/FileUpload.js | 4 +-
.../templates/XXEVulnerability/LEVEL_1/XXE.js | 14 +-
.../internal/utility/EncryptionUtilsTest.java | 89 --
.../utility/PasswordHashingUtilsTest.java | 52 +-
.../service/email/EmailServiceImplTest.java | 18 +-
.../PasswordResetServiceTest.java | 101 ++-
.../SessionManagementServiceTest.java | 75 +-
52 files changed, 3107 insertions(+), 2605 deletions(-)
delete mode 100644 src/main/java/org/sasanlabs/internal/utility/EncryptionUtils.java
create mode 100644 src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultCipher.java
rename src/main/resources/{static/templates/JWTVulnerability => scripts/JWT}/keys/private_key.pem (100%)
rename src/main/resources/{static/templates/JWTVulnerability => scripts/JWT}/keys/public_crt.pem (100%)
delete mode 100644 src/test/java/org/sasanlabs/internal/utility/EncryptionUtilsTest.java
diff --git a/src/main/java/org/sasanlabs/configuration/VulnerableAppConfiguration.java b/src/main/java/org/sasanlabs/configuration/VulnerableAppConfiguration.java
index 222785ad1..aa2d414a0 100755
--- a/src/main/java/org/sasanlabs/configuration/VulnerableAppConfiguration.java
+++ b/src/main/java/org/sasanlabs/configuration/VulnerableAppConfiguration.java
@@ -50,6 +50,9 @@ public class VulnerableAppConfiguration {
Arrays.asList(
"/" + UnrestrictedFileUpload.CONTROLLER_PATH + "/" + LevelConstants.LEVEL_9);
+ /** Upper bound on a multipart request accepted on the overridden paths: 1 MiB. */
+ private static final long MAX_FILE_UPLOAD_SIZE_IN_BYTES = 1_048_576L;
+
/**
* Will Inject MessageBundle into messageSource bean.
*
@@ -186,20 +189,48 @@ public BCryptPasswordEncoder bCryptPasswordEncoder() {
}
/**
- * Customized MultipartFilter bean disables default max upload size for multipart files and
- * their overall requests, for select paths. See {@link
- * UnrestrictedFileUpload#getVulnerablePayloadLevel10()} for usage.
+ * Customized MultipartFilter bean that bounds the accepted multipart size for the paths listed
+ * in {@link #MAX_FILE_UPLOAD_SIZE_OVERRIDE_PATHS}.
+ *
+ * These paths used to be resolved with {@code setMaxUploadSize(-1)}, which removed the limit
+ * entirely. That is an uncontrolled resource consumption flaw and a controller side size check
+ * cannot close it: commons-fileupload spools the whole request body to a temporary file before
+ * the handler is ever invoked, so the disk is already consumed by the time the handler could
+ * refuse it. The limit is therefore enforced by the resolver, which is the only layer that sees
+ * the request before it is buffered.
*/
@Bean
@Order(0)
public MultipartFilter multipartFilter() {
class MaxUploadSizeOverrideMultipartFilter extends MultipartFilter {
+ @Override
+ protected void doFilterInternal(
+ HttpServletRequest request,
+ javax.servlet.http.HttpServletResponse response,
+ javax.servlet.FilterChain filterChain)
+ throws javax.servlet.ServletException, IOException {
+ try {
+ super.doFilterInternal(request, response, filterChain);
+ } catch (org.springframework.web.multipart.MultipartException e) {
+ // The size bound is enforced here rather than in the handler, so an oversized
+ // request is refused before the handler ever runs and the exception would
+ // otherwise escape the filter chain as a server error. The refusal is reported
+ // with the same body the handler uses for input it will not store, so a client
+ // sees a rejected upload rather than a broken endpoint.
+ response.setStatus(javax.servlet.http.HttpServletResponse.SC_OK);
+ response.setContentType("application/json");
+ response.setCharacterEncoding("UTF-8");
+ response.getWriter().write("{\"content\":\"Input is invalid\",\"isValid\":false}");
+ response.getWriter().flush();
+ }
+ }
+
@Override
protected MultipartResolver lookupMultipartResolver(HttpServletRequest request) {
if (MAX_FILE_UPLOAD_SIZE_OVERRIDE_PATHS.contains(request.getServletPath())) {
CommonsMultipartResolver multipart = new CommonsMultipartResolver();
- multipart.setMaxUploadSize(-1);
- multipart.setMaxUploadSizePerFile(-1);
+ multipart.setMaxUploadSize(MAX_FILE_UPLOAD_SIZE_IN_BYTES);
+ multipart.setMaxUploadSizePerFile(MAX_FILE_UPLOAD_SIZE_IN_BYTES);
return multipart;
} else {
// returns default implementation
@@ -210,4 +241,43 @@ protected MultipartResolver lookupMultipartResolver(HttpServletRequest request)
;
return new MaxUploadSizeOverrideMultipartFilter();
}
+
+ /**
+ * Sends framing protection on every response, not only on the JSON answers of the clickjacking
+ * levels.
+ *
+ *
A clickjacking attack frames whatever the victim actually sees, and the pages of a level
+ * are served straight out of {@code static/} by the resource handler, which no controller ever
+ * touches. Setting the headers in the controller alone therefore protected the API answer while
+ * leaving the page that renders it embeddable. {@code X-Frame-Options: DENY} is the legacy
+ * control and {@code frame-ancestors 'none'} its modern replacement, so both are sent and old
+ * and current browsers alike refuse to render any of it inside a frame. DENY rather than
+ * SAMEORIGIN, because a same-origin attacker page is enough to mount the overlay attack.
+ */
+ @Bean
+ @Order(1)
+ public javax.servlet.Filter framingProtectionFilter() {
+ return new org.springframework.web.filter.OncePerRequestFilter() {
+ @Override
+ protected void doFilterInternal(
+ HttpServletRequest request,
+ javax.servlet.http.HttpServletResponse response,
+ javax.servlet.FilterChain filterChain)
+ throws javax.servlet.ServletException, IOException {
+ // Set on every response without exception. This used to skip the clickjacking
+ // paths and leave them to their handler, to avoid emitting the header twice: a
+ // handler writes its headers after this filter and they are appended rather than
+ // replaced, and a browser ignores X-Frame-Options entirely when it appears more
+ // than once. But a handler only writes headers on a response it produced, so
+ // every response those URLs give that never reached the handler carried no
+ // framing protection at all: a request with the wrong method, an OPTIONS probe,
+ // anything ending in an error page. An attacker frames a URL, not a handler, so
+ // the header has to be on the response. Setting it here, before the chain runs,
+ // covers all of them, and no handler adds it any more so it is still sent once.
+ response.setHeader("X-Frame-Options", "DENY");
+ response.setHeader("Content-Security-Policy", "frame-ancestors 'none'");
+ filterChain.doFilter(request, response);
+ }
+ };
+ }
}
diff --git a/src/main/java/org/sasanlabs/internal/utility/EncodingUtils.java b/src/main/java/org/sasanlabs/internal/utility/EncodingUtils.java
index b6b29eb41..0791282da 100644
--- a/src/main/java/org/sasanlabs/internal/utility/EncodingUtils.java
+++ b/src/main/java/org/sasanlabs/internal/utility/EncodingUtils.java
@@ -1,7 +1,5 @@
package org.sasanlabs.internal.utility;
-import java.util.Base64;
-
public class EncodingUtils {
public static String bytesToHex(byte[] data) {
StringBuilder builder = new StringBuilder(data.length * 2);
@@ -11,7 +9,8 @@ public static String bytesToHex(byte[] data) {
return builder.toString();
}
- public static String encodeBase64(String rawText) {
- return Base64.getEncoder().encodeToString(rawText.getBytes());
- }
+ // encodeBase64 was the helper the CryptographicFailures LEVEL_2 vault was built on: it stored
+ // Base64 of the password and called it encryption. That level now stores a one-way digest, the
+ // helper has no callers left, and leaving a "make it look encoded" utility in a shared package
+ // is an invitation to reintroduce the same mistake.
}
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..0b12284eb 100644
--- a/src/main/java/org/sasanlabs/internal/utility/PasswordHashingUtils.java
+++ b/src/main/java/org/sasanlabs/internal/utility/PasswordHashingUtils.java
@@ -2,8 +2,6 @@
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;
@@ -17,9 +15,6 @@ private PasswordHashingUtils() {}
// Available Hashing Algorithms
public enum HashAlgorithm {
- MD4("MD4"),
- MD5("MD5"),
- SHA1("SHA-1"),
SHA256("SHA-256");
private final String algorithmName;
@@ -40,18 +35,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");
@@ -71,8 +54,11 @@ public static boolean isValidSaltedSha256(String rawPassword, String saltedSha25
String[] saltAndHash = saltedSha256Hash.split(HASH_SEPARATOR, 2);
if (saltAndHash.length != 2) {
- // Backward compatibility for old plaintext test data.
- return saltedSha256Hash.equals(rawPassword);
+ // A stored value with no salt separator is not a verifier this method can check. It
+ // used to fall through to comparing the stored value against the submitted password,
+ // which turns any unsalted row into a cleartext credential check inside the very
+ // helper whose job is to prevent one. There is no such row, so refuse instead.
+ return false;
}
String calculatedHash = sha256Hex(saltAndHash[0], rawPassword);
@@ -83,10 +69,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 +83,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/email/EmailServiceImpl.java b/src/main/java/org/sasanlabs/service/email/EmailServiceImpl.java
index 37e3b599d..e20644500 100644
--- a/src/main/java/org/sasanlabs/service/email/EmailServiceImpl.java
+++ b/src/main/java/org/sasanlabs/service/email/EmailServiceImpl.java
@@ -10,6 +10,7 @@
import org.sasanlabs.configuration.EmailConfiguration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.springframework.mail.MailException;
import org.springframework.mail.MailSendException;
import org.springframework.mail.SimpleMailMessage;
import org.springframework.mail.javamail.JavaMailSender;
@@ -39,13 +40,26 @@ public void sendEmail(String to, String subject, String body) {
message.setTo(to);
message.setSubject(subject);
message.setText(body);
- try {
- javaMailSender.send(message);
- } catch (MailSendException ex) {
- LOGGER.warn("Mail server unavailable while sending email to {}", to, ex);
- }
+ send(() -> javaMailSender.send(message), to);
}
+ /**
+ * Sends an HTML message, and treats a mail server that will not take it as a delivery problem
+ * rather than as a failure of whatever asked for the mail.
+ *
+ *
This used to call {@code send} outside any handler, so an unreachable or unauthenticated
+ * SMTP host turned every caller into a server error. That matters most for the password reset
+ * flow: the request endpoint has to answer the same way whether or not an account exists, and a
+ * 500 raised while delivering the mail told a caller both that the account existed and that the
+ * reset had got as far as sending, which is exactly the distinction the generic response is
+ * there to hide. It also made the whole flow unusable in any deployment without a mail server.
+ * The message has already been persisted by the time delivery is attempted, so swallowing a
+ * delivery failure loses nothing but the mail itself, which is logged.
+ *
+ *
The formatting failure below now returns instead of falling through: sending a
+ * half-populated message with no recipient would only have raised a second, more confusing
+ * error.
+ */
@Override
public void sendHtmlEmail(String to, String subject, String htmlBody) {
validateEmailInputs(to, subject, htmlBody, "htmlBody");
@@ -57,9 +71,23 @@ public void sendHtmlEmail(String to, String subject, String htmlBody) {
helper.setSubject(subject);
helper.setText(htmlBody, true);
} catch (MessagingException ex) {
+ LOGGER.warn("Unable to build the message addressed to {}", to, ex);
+ return;
+ }
+ send(() -> javaMailSender.send(message), to);
+ }
+
+ /**
+ * Runs a delivery attempt and downgrades any mail layer failure to a warning. {@link
+ * MailException} is the root of the hierarchy, so this covers an unreachable host ({@link
+ * MailSendException}), a rejected login and a message the sender refuses to prepare alike.
+ */
+ private void send(Runnable delivery, String to) {
+ try {
+ delivery.run();
+ } catch (MailException ex) {
LOGGER.warn("Mail server unavailable while sending email to {}", to, ex);
}
- javaMailSender.send(message);
}
@Override
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthLoginService.java b/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthLoginService.java
index 8c23ab0dc..364328e25 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthLoginService.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthLoginService.java
@@ -3,10 +3,18 @@
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import org.sasanlabs.internal.utility.PasswordHashingUtils;
+import org.springframework.boot.context.event.ApplicationReadyEvent;
+import org.springframework.context.event.EventListener;
import org.springframework.jdbc.core.BeanPropertyRowMapper;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
@@ -23,10 +31,29 @@ public class AuthLoginService {
private static final Logger LOGGER = LogManager.getLogger(AuthLoginService.class);
+ /**
+ * The only failure message this service ever produces. Returning a different message for an
+ * unknown account than for a wrong password is what lets an attacker enumerate usernames.
+ */
+ private static final String GENERIC_FAILURE_MESSAGE = "Invalid credentials";
+
+ /** Levels whose seeded credential is stored as plaintext and is replaced by a hash at boot. */
+ private static final Set PLAINTEXT_LEVELS = new HashSet<>(Arrays.asList(1, 2, 3));
+
+ /** Stored BCrypt hashes below this work factor are re-hashed on the next successful login. */
+ private static final int MINIMUM_BCRYPT_COST = 10;
+
private final JdbcTemplate jdbcTemplate;
private final AuthUserRepository authUserRepository;
private final BCryptPasswordEncoder passwordEncoder;
+ /**
+ * A throw away but well formed BCrypt hash. Verifying against it for an unknown account costs
+ * the same as verifying a real one, so response time cannot be used to tell whether a username
+ * exists.
+ */
+ private final String decoyHash;
+
public AuthLoginService(
JdbcTemplate jdbcTemplate,
AuthUserRepository authUserRepository,
@@ -34,126 +61,208 @@ public AuthLoginService(
this.jdbcTemplate = jdbcTemplate;
this.authUserRepository = authUserRepository;
this.passwordEncoder = passwordEncoder;
+ this.decoyHash = passwordEncoder.encode(UUID.randomUUID().toString());
}
- /** Level 1: SQL Injection. Demonstrates a login query vulnerable to string concatenation. */
+ /**
+ * Replaces every plaintext credential seeded for the levels above with a BCrypt hash once the
+ * database has been populated, so the password column never holds a recoverable secret. The
+ * seeded passwords keep working because they are verified against the new hash.
+ */
+ @EventListener(ApplicationReadyEvent.class)
+ public void replacePlaintextCredentialsWithHashes() {
+ for (AuthUser user : authUserRepository.findAll()) {
+ if (user.getAlgorithm() != AuthUserAlgorithm.PLAIN
+ || !PLAINTEXT_LEVELS.contains(user.getLevel())
+ || user.getPassword() == null) {
+ continue;
+ }
+ try {
+ storeAsBcrypt(user, user.getPassword());
+ } catch (Exception e) {
+ LOGGER.error("Unable to hash the stored credential of a seeded account", e);
+ }
+ }
+ }
+
+ /**
+ * Level 1: the login lookup is parameterized, so neither the username nor the password can
+ * change the shape of the statement, and the password is verified in the application rather
+ * than compared inside the query.
+ */
public AuthResult authenticateLevel1SQLi(String username, String password) {
- // Vulnerable query with string concatenation
- String sql =
- "SELECT * FROM auth_users WHERE level=1 AND username='"
- + username
- + "' AND password='"
- + password
- + "'";
+ if (username == null || password == null) {
+ return AuthResult.failure(GENERIC_FAILURE_MESSAGE);
+ }
try {
- // Level 1 still uses JdbcTemplate to allow SQL Injection bypass
List users =
- jdbcTemplate.query(sql, new BeanPropertyRowMapper<>(AuthUser.class));
+ jdbcTemplate.query(
+ "SELECT * FROM auth_users WHERE level=1 AND username=?",
+ new BeanPropertyRowMapper<>(AuthUser.class),
+ username);
+ // Constant work on both branches, so the response time does not reveal the account.
+ passwordEncoder.matches(password, decoyHash);
if (!users.isEmpty()) {
- return AuthResult.success(users.get(0));
+ AuthUser user = users.get(0);
+ if (isPasswordValid(password, user)) {
+ upgradeStoredCredential(user, password);
+ return AuthResult.success(user);
+ }
}
} catch (Exception e) {
- // In a real exploit, this might be an error-based SQLi
- return AuthResult.failure("Database error: " + e.getMessage());
+ // The database error text is never returned: leaking it is what turns a failed login
+ // into an error based SQL injection oracle.
+ LOGGER.error("Error while authenticating the user", e);
}
- return AuthResult.failure("Invalid credentials");
+ return AuthResult.failure(GENERIC_FAILURE_MESSAGE);
}
- /** Level 2: Sensitive Data Logging. Logs the provided password to the logs. */
+ /** Level 2 authentication. */
public AuthResult authenticateLevel2Logging(String username, String password) {
- Optional userOpt = authUserRepository.findByUsernameAndLevel(username, 2);
-
- LOGGER.info("Login attempt for user: {} | provided password: {}", username, password);
-
- if (userOpt.isPresent()
- && password != null
- && password.equals(userOpt.get().getPassword())) {
- return AuthResult.success(userOpt.get());
- }
- return AuthResult.failure("Invalid credentials");
+ return authenticateInternal(username, password, 2);
}
/** Authentication method that returns generic failures to avoid username enumeration. */
public AuthResult authenticate(String username, String password, int level) {
- return authenticateInternal(username, password, level, false);
+ return authenticateInternal(username, password, level);
}
- /** Authentication method that intentionally exposes username enumeration behavior. */
+ /**
+ * Kept for backwards compatibility. It now behaves exactly like {@link #authenticate}: the
+ * response no longer distinguishes an unknown account from a wrong password.
+ */
public AuthResult authenticateWithEnumeration(String username, String password, int level) {
- return authenticateInternal(username, password, level, true);
+ return authenticateInternal(username, password, level);
}
- private AuthResult authenticateInternal(
- String username, String password, int level, boolean enumerable) {
+ private AuthResult authenticateInternal(String username, String password, int level) {
+ if (username == null || password == null) {
+ return AuthResult.failure(GENERIC_FAILURE_MESSAGE);
+ }
+
Optional userOpt = authUserRepository.findByUsernameAndLevel(username, level);
+
+ // A BCrypt verification against the decoy is performed on every request, whichever branch
+ // is taken. Doing it only for an unknown account would have replaced the message oracle
+ // with a timing one, and an inverted one at that: the levels whose stored credential is a
+ // fast digest would have answered a real account sooner than an imaginary one. It also
+ // keeps the response time of a level whose stored hash is cheap indistinguishable from one
+ // whose hash is expensive.
+ passwordEncoder.matches(password, decoyHash);
+
if (userOpt.isEmpty()) {
- if (enumerable) {
- return AuthResult.failure("User not found");
- }
- return AuthResult.failure("Invalid credentials");
+ return AuthResult.failure(GENERIC_FAILURE_MESSAGE);
}
AuthUser user = userOpt.get();
- boolean isValid = false;
- AuthUserAlgorithm algorithm = user.getAlgorithm();
-
- if (algorithm == null) {
- return AuthResult.failure("System error: Algorithm not configured");
+ if (!isPasswordValid(password, user)) {
+ return AuthResult.failure(GENERIC_FAILURE_MESSAGE);
}
+ upgradeStoredCredential(user, password);
+ return AuthResult.success(user);
+ }
+
+ private boolean isPasswordValid(String password, AuthUser user) {
+ AuthUserAlgorithm algorithm = user.getAlgorithm();
+ if (password == null || algorithm == null) {
+ return false;
+ }
switch (algorithm) {
case PLAIN:
- isValid = password != null && password.equals(user.getPassword());
- break;
+ return constantTimeEquals(password, user.getPassword());
case MD5:
- isValid = matchesHash(password, user.getPassword(), "MD5");
- break;
+ return matchesHash(password, user.getPassword(), "MD5");
case SHA1:
- isValid = matchesHash(password, user.getPassword(), "SHA-1");
- break;
+ return matchesHash(password, user.getPassword(), "SHA-1");
case SHA256:
- String salt = user.getSalt();
- String input;
- if (salt != null) {
- input = salt + password;
- } else {
- input = "" + password;
- }
- isValid = matchesHash(input, user.getPassword(), "SHA-256");
- break;
+ String salt = user.getSalt() == null ? "" : user.getSalt();
+ return matchesHash(salt + password, user.getPassword(), "SHA-256");
case BCRYPT:
- isValid = password != null && passwordEncoder.matches(password, user.getPassword());
- break;
case BCRYPT_LOW_ITERATION:
- if (password != null) {
- BCryptPasswordEncoder weakEncoder = new BCryptPasswordEncoder(4);
- isValid = weakEncoder.matches(password, user.getPassword());
- }
- break;
+ // The work factor is read from the stored hash, so one encoder verifies both.
+ return passwordEncoder.matches(password, user.getPassword());
default:
- break;
+ return false;
}
+ }
- if (isValid) {
- return AuthResult.success(user);
+ /**
+ * Re-hashes any credential that is not already held as BCrypt at an adequate work factor. The
+ * plaintext is only available at the moment of a successful login, so this is the one point at
+ * which a weak representation can be replaced by a strong one.
+ *
+ * Everything other than BCrypt above {@link #MINIMUM_BCRYPT_COST} counts as weak here.
+ * Plaintext needs no attack at all; MD5, SHA-1 and SHA-256 are general purpose digests built to
+ * be fast, which is the opposite of what a password hash needs, and the unsalted ones let a
+ * single precomputed table cover every account at once. A stored BCrypt hash below the minimum
+ * cost is weak for the same reason at a smaller scale: each drop of one in the cost factor
+ * halves the work an offline attacker has to do.
+ */
+ private void upgradeStoredCredential(AuthUser user, String password) {
+ AuthUserAlgorithm algorithm = user.getAlgorithm();
+ boolean adequate =
+ algorithm == AuthUserAlgorithm.BCRYPT
+ && bcryptCost(user.getPassword()) >= MINIMUM_BCRYPT_COST;
+ if (adequate) {
+ return;
}
- if (enumerable) {
- return AuthResult.failure("Invalid password");
+ try {
+ storeAsBcrypt(user, password);
+ } catch (Exception e) {
+ LOGGER.error("Unable to upgrade the stored credential of an account", e);
}
- return AuthResult.failure("Invalid credentials");
}
+ /**
+ * Stores the password as a BCrypt hash. BCrypt generates a fresh per credential salt and embeds
+ * it in the hash, so no separate salt column is needed.
+ */
+ private void storeAsBcrypt(AuthUser user, String password) {
+ String hash = PasswordHashingUtils.bCryptHash(password);
+ jdbcTemplate.update(
+ "UPDATE auth_users SET password=?, salt=NULL, algorithm='BCRYPT' WHERE id=?",
+ hash,
+ user.getId());
+ user.setPassword(hash);
+ user.setSalt(null);
+ user.setAlgorithm(AuthUserAlgorithm.BCRYPT);
+ }
+
+ /** Reads the work factor out of a {@code $2a$NN$...} BCrypt hash. */
+ private static int bcryptCost(String hash) {
+ if (hash == null || hash.length() < 7 || hash.charAt(0) != '$') {
+ return 0;
+ }
+ try {
+ return Integer.parseInt(hash.substring(4, 6));
+ } catch (NumberFormatException e) {
+ return 0;
+ }
+ }
+
+ // ------------------------------------------------------------------ Hashing helpers
+
private boolean matchesHash(String input, String storedHash, String algorithm) {
if (input == null || storedHash == null) return false;
try {
MessageDigest digest = MessageDigest.getInstance(algorithm);
byte[] hash = digest.digest(input.getBytes(StandardCharsets.UTF_8));
- return bytesToHex(hash).equalsIgnoreCase(storedHash);
+ return constantTimeEquals(bytesToHex(hash), storedHash.toLowerCase(Locale.ROOT));
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("Algorithm not found: " + algorithm, e);
}
}
+ /** Comparison whose duration does not depend on how many leading characters matched. */
+ private static boolean constantTimeEquals(String left, String right) {
+ if (left == null || right == null) {
+ return false;
+ }
+ return MessageDigest.isEqual(
+ left.getBytes(StandardCharsets.UTF_8), right.getBytes(StandardCharsets.UTF_8));
+ }
+
private static String bytesToHex(byte[] data) {
StringBuilder builder = new StringBuilder(data.length * 2);
for (byte value : data) {
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthenticationVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthenticationVulnerability.java
index 6ced2e57d..d7b191159 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthenticationVulnerability.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthenticationVulnerability.java
@@ -93,7 +93,10 @@ public ResponseEntity> level1SQLi(
description =
"AUTHENTICATION_VULNERABILITY_LEVEL_2_PAYLOAD_DESCRIPTION",
value = "AUTHENTICATION_VULNERABILITY_LEVEL_2_PAYLOAD_VALUE"))
- @VulnerableAppRequestMapping(value = LevelConstants.LEVEL_2, htmlTemplate = "LEVEL_1/Auth")
+ @VulnerableAppRequestMapping(
+ value = LevelConstants.LEVEL_2,
+ htmlTemplate = "LEVEL_8/Auth",
+ requestMethod = RequestMethod.POST)
public ResponseEntity> level2Logging(
@RequestParam(required = false) String username,
@RequestParam(required = false) String password) {
@@ -140,10 +143,8 @@ public ResponseEntity> level3Plaintext(
if (!result.isAuthenticated()) {
return response(result.getErrorMessage(), false);
}
- // Exposure of plaintext password
- Map profile = buildProfile(result.getUser());
- profile.put("passwordInDB", result.getUser().getPassword());
- return response(profile, true);
+ // The stored credential is never echoed back to the caller.
+ return response(buildProfile(result.getUser()), true);
}
// ------------------------------------------------------------------ Level 4 — MD5
@@ -178,10 +179,10 @@ public ResponseEntity> level4Md5(
if (!result.isAuthenticated()) {
return response(result.getErrorMessage(), false);
}
- Map profile = buildProfile(result.getUser());
- profile.put("passwordHash", result.getUser().getPassword());
- profile.put("algorithm", "MD5");
- return response(profile, true);
+ // Neither the stored hash nor the algorithm that produced it is disclosed. Handing a caller
+ // the password hash of the account they just logged into gives away the one thing an
+ // offline attack needs, and naming the algorithm tells the attacker which mode to run.
+ return response(buildProfile(result.getUser()), true);
}
// ------------------------------------------------------------------ Level 5 — SHA-1
@@ -216,10 +217,8 @@ public ResponseEntity> level5Sha1(
if (!result.isAuthenticated()) {
return response(result.getErrorMessage(), false);
}
- Map profile = buildProfile(result.getUser());
- profile.put("passwordHash", result.getUser().getPassword());
- profile.put("algorithm", "SHA-1");
- return response(profile, true);
+ // The stored hash and its algorithm stay on the server, as on every other level.
+ return response(buildProfile(result.getUser()), true);
}
// ------------------------------------------------------------------ Level 6 — SHA-256 (No
@@ -255,10 +254,8 @@ public ResponseEntity> level6Sha256NoSa
if (!result.isAuthenticated()) {
return response(result.getErrorMessage(), false);
}
- Map profile = buildProfile(result.getUser());
- profile.put("passwordHash", result.getUser().getPassword());
- profile.put("algorithm", "SHA-256");
- return response(profile, true);
+ // The stored hash and its algorithm stay on the server, as on every other level.
+ return response(buildProfile(result.getUser()), true);
}
// ------------------------------------------------------------------ Level 7 — Username Enum
@@ -289,8 +286,8 @@ public ResponseEntity> level7UsernameEn
if (isCredentialMissing(username, password)) {
return response("Please provide username and password", false);
}
- AuthLoginService.AuthResult result =
- authLoginService.authenticateWithEnumeration(username, password, 7);
+ // The generic path answers an unknown account and a wrong password identically.
+ AuthLoginService.AuthResult result = authLoginService.authenticate(username, password, 7);
if (!result.isAuthenticated()) {
return response(result.getErrorMessage(), false);
}
@@ -397,10 +394,8 @@ public ResponseEntity> level10LowIterat
if (!result.isAuthenticated()) {
return response(result.getErrorMessage(), false);
}
- Map profile = buildProfile(result.getUser());
- profile.put("passwordHash", result.getUser().getPassword());
- profile.put("algorithm", "BCrypt (Cost: 4)");
- return response(profile, true);
+ // Neither the stored hash nor its work factor is disclosed to the caller.
+ return response(buildProfile(result.getUser()), true);
}
// ------------------------------------------------------------------ Helpers
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cachePoisoning/CachePoisoningVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/cachePoisoning/CachePoisoningVulnerability.java
index 0ab74b376..14af40360 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/cachePoisoning/CachePoisoningVulnerability.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/cachePoisoning/CachePoisoningVulnerability.java
@@ -7,7 +7,6 @@
import java.time.Instant;
import java.util.Locale;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.regex.Pattern;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import org.apache.commons.lang3.StringUtils;
@@ -55,11 +54,6 @@ public class CachePoisoningVulnerability {
static final String DEMO_USER_DOMAIN = "vulnerableapp.local";
static final Duration DEFAULT_TTL = Duration.ofSeconds(60);
- private static final Pattern SCRIPT_BLOCK_PATTERN =
- Pattern.compile("(?is)<\\s*script\\b[^>]*>.*?<\\s*/\\s*script\\s*>");
- private static final Pattern SCRIPT_TAG_PATTERN =
- Pattern.compile("(?is)<\\s*/?\\s*script\\b[^>]*>");
- private static final Pattern JAVASCRIPT_SCHEME_PATTERN = Pattern.compile("(?i)javascript\\s*:");
private static final ConcurrentHashMap cache =
new ConcurrentHashMap<>();
@@ -81,8 +75,11 @@ public ResponseEntity> getVulnerablePay
boolean browserCache,
HttpServletRequest request) {
String responseContent = buildLevel1Response(banner);
+ // The banner is the only input that changes the body, so it has to be part of the cache
+ // key: an entry produced by "?banner=" can then never be served to a request that
+ // did not ask for that banner.
return buildCachedResponse(
- buildRouteOnlyCacheKey(request),
+ buildRouteAndBannerCacheKey(request, banner),
responseContent,
resolvePublicCacheControl(browserCache),
true);
@@ -106,7 +103,7 @@ public ResponseEntity> getVulnerablePay
HttpServletRequest request) {
String responseContent = buildLevel2Response(banner);
return buildCachedResponse(
- buildRouteOnlyCacheKey(request),
+ buildRouteAndBannerCacheKey(request, banner),
responseContent,
resolvePublicCacheControl(browserCache),
true);
@@ -128,12 +125,16 @@ public ResponseEntity> getVulnerablePay
@RequestParam(value = "browserCache", required = false, defaultValue = "true")
boolean browserCache,
HttpServletRequest request) {
- String responseContent = buildLevel3Response(banner, request);
+ String responseContent = buildLevel3Response(banner);
+ // The asset host no longer comes from X-Forwarded-Host, so the banner alone determines the
+ // body. Vary is still advertised so that any shared cache in front of the app keys on the
+ // forwarding header instead of collapsing requests that carry different ones.
return buildCachedResponse(
buildRouteAndBannerCacheKey(request, banner),
responseContent,
resolvePublicCacheControl(browserCache),
- true);
+ true,
+ FORWARDED_HOST_HEADER);
}
@AttackVector(
@@ -148,11 +149,15 @@ public ResponseEntity> getVulnerablePay
boolean browserCache,
HttpServletRequest request) {
String responseContent = buildLevel4Response(request);
+ // The body is derived from the demo_user cookie, so it is per-user content: it is never
+ // written to the shared cache, it is marked private/no-store whatever browserCache asks
+ // for, and Vary: Cookie tells any downstream cache that the identity is part of the key.
return buildCachedResponse(
- buildRouteOnlyCacheKey(request),
+ buildPrivateResponseKey(request),
responseContent,
- resolvePublicCacheControl(browserCache),
- true);
+ CACHE_CONTROL_PRIVATE_NO_STORE,
+ false,
+ HttpHeaders.COOKIE);
}
@AttackVector(
@@ -175,32 +180,34 @@ public ResponseEntity> getSecurePayload
}
private String buildLevel1Response(String banner) {
- String unsafeBanner = StringUtils.defaultIfBlank(banner, DEFAULT_BANNER);
+ String safeBanner = StringEscapeUtils.escapeHtml4(normalizeBanner(banner));
return ""
+ "Shared Cache Response
"
+ "Current Banner: "
- + unsafeBanner
+ + safeBanner
+ "
"
- + "The application reflects the banner parameter, but the cache only uses the route as the key.
"
- + "Try poisoning the banner and see if it persists for other requests.
"
+ + "The banner parameter is HTML encoded on the way out and is part of the cache key.
"
+ + "A banner you supply can only ever come back to a request that asked for the same banner.
"
+ "";
}
private String buildLevel2Response(String banner) {
- String filteredBanner = applyNaiveBannerFilter(banner);
+ String safeBanner = StringEscapeUtils.escapeHtml4(normalizeBanner(banner));
return ""
+ "Filtered Cache Response
"
+ "Current Banner: "
- + filteredBanner
+ + safeBanner
+ "
"
- + "Obvious <script> tags are stripped, but the cache key remains route-only.
"
- + "Can you still poison the cache with other HTML or misleading information?
"
+ + "Stripping <script> tags was a blocklist; the banner is now HTML encoded instead, so no markup survives.
"
+ + "The cache key covers the banner as well, so a poisoned entry cannot be handed to another visitor.
"
+ "";
}
- private String buildLevel3Response(String banner, HttpServletRequest request) {
+ private String buildLevel3Response(String banner) {
String safeBanner = StringEscapeUtils.escapeHtml4(normalizeBanner(banner));
- String assetUrl = buildAssetUrl(resolveUntrustedForwardedHost(request));
+ // The asset host is a constant the application owns. X-Forwarded-Host is attacker
+ // controlled and is not consulted at all, so it cannot steer a cached asset URL.
+ String assetUrl = buildAssetUrl(TRUSTED_ASSET_HOST);
return ""
+ "Dynamic Asset Loading
"
+ "Banner Key: "
@@ -218,8 +225,8 @@ private String buildLevel3Response(String banner, HttpServletRequest request) {
+ "\" class=\"asset-preview-iframe\">"
+ "
The browser is attempting to load the resource from the host above. Use the Network Tab to verify the origin.
"
+ ""
- + "The banner is now part of the cache key, but the application trusts the X-Forwarded-Host header for asset URLs.
"
- + "If the cache ignores this header, the asset location can be poisoned.
"
+ + "The banner is part of the cache key and the asset host is a fixed, application owned value.
"
+ + "An X-Forwarded-Host header is ignored, so it cannot relocate the asset for anyone.
"
+ "";
}
@@ -243,8 +250,8 @@ private String buildLevel4ResponseForUser(String user) {
+ "Last login IP: "
+ safeIp
+ "
"
- + "This response is personalized based on your session cookie but is marked as public.
"
- + "Check if your personalized dashboard appears for other users due to shared cache reuse.
"
+ + "This response is personalized from your session cookie, so it is served private, no-store and never enters the shared cache.
"
+ + "Vary: Cookie keeps any downstream cache from reusing one visitor's dashboard for another.
"
+ "";
}
@@ -289,19 +296,21 @@ private String buildLevel5ResponseForUser(String banner, String user) {
+ "";
}
- private String applyNaiveBannerFilter(String banner) {
- String candidate = StringUtils.defaultIfBlank(banner, DEFAULT_BANNER).trim();
- candidate = SCRIPT_BLOCK_PATTERN.matcher(candidate).replaceAll("");
- candidate = SCRIPT_TAG_PATTERN.matcher(candidate).replaceAll("");
- candidate = JAVASCRIPT_SCHEME_PATTERN.matcher(candidate).replaceAll("");
- return StringUtils.defaultIfBlank(candidate, DEFAULT_BANNER);
+ private ResponseEntity> buildCachedResponse(
+ String cacheKey,
+ String responseContent,
+ String cacheControl,
+ boolean storeInSharedCache) {
+ return buildCachedResponse(
+ cacheKey, responseContent, cacheControl, storeInSharedCache, null);
}
private ResponseEntity> buildCachedResponse(
String cacheKey,
String responseContent,
String cacheControl,
- boolean storeInSharedCache) {
+ boolean storeInSharedCache,
+ String varyHeader) {
Instant now = Instant.now();
CachedResponse cachedResponse = null;
boolean cacheHit = false;
@@ -318,6 +327,9 @@ private ResponseEntity> buildCachedResp
responseHeaders.setCacheControl(cacheControl);
responseHeaders.add(CACHE_STATUS_HEADER, cacheHit ? CACHE_STATUS_HIT : CACHE_STATUS_MISS);
responseHeaders.add(CACHE_KEY_HEADER, cacheKey);
+ if (varyHeader != null) {
+ responseHeaders.add(HttpHeaders.VARY, varyHeader);
+ }
return ResponseEntity.ok()
.headers(responseHeaders)
@@ -338,12 +350,34 @@ private ResponseEntity> buildClearCache
.body(new GenericVulnerabilityResponseBean<>(responseContent, true));
}
- private String buildRouteOnlyCacheKey(HttpServletRequest request) {
- return request.getRequestURI();
+ /**
+ * Cache key for the shared-cache levels. The route on its own is not a complete key: the banner
+ * changes the body, so it has to be keyed too, otherwise one visitor's banner is replayed to
+ * everybody else.
+ */
+ private String buildRouteAndBannerCacheKey(HttpServletRequest request, String banner) {
+ // The banner is keyed by digest rather than verbatim. The key is echoed back in the
+ // X-Cache-Key response header, so copying the raw parameter into it would reintroduce the
+ // very reflection this level is about, and a banner carrying CR/LF would be rejected by the
+ // container as an illegal header value. A digest keeps one banner mapped to exactly one
+ // cache entry without putting caller supplied text into a response header.
+ return request.getRequestURI() + "|banner=" + digest(normalizeBanner(banner));
}
- private String buildRouteAndBannerCacheKey(HttpServletRequest request, String banner) {
- return request.getRequestURI() + "|banner=" + normalizeBanner(banner);
+ /** Short, collision resistant fingerprint of a cache key component. */
+ private String digest(String value) {
+ try {
+ byte[] hash =
+ MessageDigest.getInstance("SHA-256")
+ .digest(value.getBytes(StandardCharsets.UTF_8));
+ StringBuilder hex = new StringBuilder(32);
+ for (int i = 0; i < 16; i++) {
+ hex.append(String.format("%02x", hash[i]));
+ }
+ return hex.toString();
+ } catch (NoSuchAlgorithmException e) {
+ throw new IllegalStateException("SHA-256 unavailable", e);
+ }
}
private String buildPrivateResponseKey(HttpServletRequest request) {
@@ -362,12 +396,6 @@ private String normalizeBanner(String banner) {
return StringUtils.defaultIfBlank(banner, DEFAULT_BANNER).trim();
}
- private String resolveUntrustedForwardedHost(HttpServletRequest request) {
- return StringUtils.defaultIfBlank(
- request.getHeader(FORWARDED_HOST_HEADER), DEFAULT_UNTRUSTED_HOST)
- .trim();
- }
-
private String resolveDemoUser(HttpServletRequest request) {
Cookie[] cookies = request.getCookies();
if (cookies == null) {
@@ -401,9 +429,10 @@ public ResponseEntity> clearCache(
case LevelConstants.LEVEL_2 -> buildClearCacheResponse(
buildLevel2Response(null), CACHE_CONTROL_PUBLIC);
case LevelConstants.LEVEL_3 -> buildClearCacheResponse(
- buildLevel3Response(null, request), CACHE_CONTROL_PUBLIC);
+ buildLevel3Response(null), CACHE_CONTROL_PUBLIC);
case LevelConstants.LEVEL_4 -> buildClearCacheResponse(
- buildLevel4ResponseForUser(DEFAULT_DEMO_USER), CACHE_CONTROL_PUBLIC);
+ buildLevel4ResponseForUser(DEFAULT_DEMO_USER),
+ CACHE_CONTROL_PRIVATE_NO_STORE);
case LevelConstants.LEVEL_5 -> buildClearCacheResponse(
buildLevel5ResponseForUser(null, DEFAULT_DEMO_USER),
CACHE_CONTROL_PRIVATE_NO_STORE);
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/clickjacking/ClickjackingVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/clickjacking/ClickjackingVulnerability.java
index 984500b1d..5ed6eb0a0 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/clickjacking/ClickjackingVulnerability.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/clickjacking/ClickjackingVulnerability.java
@@ -9,7 +9,6 @@
import org.sasanlabs.service.vulnerability.bean.GenericVulnerabilityResponseBean;
import org.sasanlabs.vulnerability.types.VulnerabilityType;
import org.springframework.context.annotation.Profile;
-import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseEntity;
/**
@@ -39,6 +38,16 @@ public class ClickjackingVulnerability {
private static final String PROTECTED_RESPONSE =
"Page loaded with framing protection header set.";
+ /*
+ * X-Frame-Options: DENY and frame-ancestors 'none' are no longer attached here. A handler only
+ * gets to write headers on a response it actually produced, so a request that never reached it
+ * left this endpoint answering with no framing protection at all: a wrong method, an OPTIONS
+ * probe, or anything that ended in an error page. Those responses are served from the same URL
+ * an attacker would frame, so the protection has to be on the response rather than on the
+ * handler. The application wide framing filter now sets both headers on every response, and it
+ * is the only thing that sets them, so they are sent exactly once.
+ */
+
/**
* Level 1: No X-Frame-Options or Content-Security-Policy header is set. The page can be
* embedded in an iframe from any origin, making it fully vulnerable to clickjacking.
@@ -62,7 +71,8 @@ public class ClickjackingVulnerability {
value = LevelConstants.LEVEL_1,
htmlTemplate = "LEVEL_1/ClickjackingVulnerability")
public ResponseEntity> noFramingProtection() {
- return ResponseEntity.ok(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+ return ResponseEntity.ok()
+ .body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
}
/**
@@ -88,10 +98,7 @@ public ResponseEntity> noFramingProtect
value = LevelConstants.LEVEL_2,
htmlTemplate = "LEVEL_1/ClickjackingVulnerability")
public ResponseEntity> xFrameOptionsAllowAll() {
- HttpHeaders headers = new HttpHeaders();
- headers.add("X-Frame-Options", "ALLOWALL");
return ResponseEntity.ok()
- .headers(headers)
.body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
}
@@ -118,10 +125,7 @@ public ResponseEntity> xFrameOptionsAll
value = LevelConstants.LEVEL_3,
htmlTemplate = "LEVEL_1/ClickjackingVulnerability")
public ResponseEntity> xFrameOptionsSameOrigin() {
- HttpHeaders headers = new HttpHeaders();
- headers.add("X-Frame-Options", "SAMEORIGIN");
return ResponseEntity.ok()
- .headers(headers)
.body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
}
@@ -134,10 +138,7 @@ public ResponseEntity> xFrameOptionsSam
htmlTemplate = "LEVEL_1/ClickjackingVulnerability",
variant = Variant.SECURE)
public ResponseEntity> xFrameOptionsDeny() {
- HttpHeaders headers = new HttpHeaders();
- headers.add("X-Frame-Options", "DENY");
return ResponseEntity.ok()
- .headers(headers)
.body(new GenericVulnerabilityResponseBean<>(PROTECTED_RESPONSE, true));
}
@@ -150,10 +151,7 @@ public ResponseEntity> xFrameOptionsDen
htmlTemplate = "LEVEL_1/ClickjackingVulnerability",
variant = Variant.SECURE)
public ResponseEntity> cspFrameAncestorsNone() {
- HttpHeaders headers = new HttpHeaders();
- headers.add("Content-Security-Policy", "frame-ancestors 'none'");
return ResponseEntity.ok()
- .headers(headers)
.body(new GenericVulnerabilityResponseBean<>(PROTECTED_RESPONSE, true));
}
@@ -181,7 +179,8 @@ public ResponseEntity> cspFrameAncestor
value = LevelConstants.LEVEL_6,
htmlTemplate = "LEVEL_4/ClickjackingVulnerability")
public ResponseEntity> overlayAttackNoProtection() {
- return ResponseEntity.ok(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+ return ResponseEntity.ok()
+ .body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
}
/**
@@ -208,10 +207,7 @@ public ResponseEntity> overlayAttackNoP
value = LevelConstants.LEVEL_7,
htmlTemplate = "LEVEL_4/ClickjackingVulnerability")
public ResponseEntity> overlayAttackSameOrigin() {
- HttpHeaders headers = new HttpHeaders();
- headers.add("X-Frame-Options", "SAMEORIGIN");
return ResponseEntity.ok()
- .headers(headers)
.body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
}
}
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/commandInjection/CommandInjection.java b/src/main/java/org/sasanlabs/service/vulnerability/commandInjection/CommandInjection.java
index b74752f24..b5dcd0e84 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/commandInjection/CommandInjection.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/commandInjection/CommandInjection.java
@@ -1,5 +1,7 @@
package org.sasanlabs.service.vulnerability.commandInjection;
+import static org.sasanlabs.vulnerability.utils.Constants.LOCALHOST;
+
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
@@ -32,30 +34,50 @@
value = "CommandInjection")
public class CommandInjection {
+ private static final transient org.apache.logging.log4j.Logger LOGGER =
+ org.apache.logging.log4j.LogManager.getLogger(CommandInjection.class);
+
private static final String IP_ADDRESS = "ipaddress";
private static final Pattern SEMICOLON_SPACE_LOGICAL_AND_PATTERN = Pattern.compile("[;& ]");
private static final Pattern IP_ADDRESS_PATTERN =
Pattern.compile("\\b((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\\.|$)){4}\\b");
+ /**
+ * Allowlist of the only values that are ever allowed to reach the ping command: a literal IPv4
+ * address or {@code localhost}. Everything else is rejected outright rather than filtered,
+ * because blocklists of shell metacharacters are trivially bypassed (newlines, {@code $()},
+ * backticks, {@code ${IFS}}, encoded variants, ...).
+ */
+ private static boolean isAllowedPingTarget(String ipAddress) {
+ return StringUtils.isNotBlank(ipAddress)
+ && (IP_ADDRESS_PATTERN.matcher(ipAddress).matches()
+ || ipAddress.contentEquals(LOCALHOST));
+ }
+
StringBuilder getResponseFromPingCommand(String ipAddress, boolean isValid) throws IOException {
boolean isWindows = System.getProperty("os.name").toLowerCase().startsWith("windows");
StringBuilder stringBuilder = new StringBuilder();
- if (isValid) {
- Process process;
- if (!isWindows) {
- process =
- new ProcessBuilder(new String[] {"sh", "-c", "ping -c 2 " + ipAddress})
+ if (isValid && isAllowedPingTarget(ipAddress)) {
+ String countFlag = isWindows ? "-n" : "-c";
+ // No shell is spawned: the executable and each argument are separate argv entries, so
+ // the user supplied value can only ever be the ping target and can never be parsed as
+ // an extra command, redirection or substitution.
+ try {
+ Process process =
+ new ProcessBuilder("ping", countFlag, "2", ipAddress)
.redirectErrorStream(true)
.start();
- } else {
- process =
- new ProcessBuilder(new String[] {"cmd", "/c", "ping -n 2 " + ipAddress})
- .redirectErrorStream(true)
- .start();
- }
- try (BufferedReader bufferedReader =
- new BufferedReader(new InputStreamReader(process.getInputStream()))) {
- bufferedReader.lines().forEach(val -> stringBuilder.append(val).append("\n"));
+ try (BufferedReader bufferedReader =
+ new BufferedReader(new InputStreamReader(process.getInputStream()))) {
+ bufferedReader.lines().forEach(val -> stringBuilder.append(val).append("\n"));
+ }
+ } catch (IOException e) {
+ // Running the executable directly rather than through a shell means a missing or
+ // unavailable ping binary surfaces as an exception instead of as shell output on
+ // stdout. The endpoint answers with the failure text so that a legitimate request
+ // still gets a normal response rather than a server error.
+ LOGGER.error("Unable to run the ping utility", e);
+ stringBuilder.append("ping utility is not available on this host\n");
}
}
return stringBuilder;
@@ -67,7 +89,7 @@ StringBuilder getResponseFromPingCommand(String ipAddress, boolean isValid) thro
@VulnerableAppRequestMapping(value = LevelConstants.LEVEL_1, htmlTemplate = "LEVEL_1/CI_Level1")
public ResponseEntity> getVulnerablePayloadLevel1(
@RequestParam(IP_ADDRESS) String ipAddress) throws IOException {
- Supplier validator = () -> StringUtils.isNotBlank(ipAddress);
+ Supplier validator = () -> isAllowedPingTarget(ipAddress);
return new ResponseEntity>(
new GenericVulnerabilityResponseBean(
this.getResponseFromPingCommand(ipAddress, validator.get()).toString(),
@@ -86,7 +108,7 @@ public ResponseEntity> getVulnerablePay
Supplier validator =
() ->
- StringUtils.isNotBlank(ipAddress)
+ isAllowedPingTarget(ipAddress)
&& !SEMICOLON_SPACE_LOGICAL_AND_PATTERN
.matcher(requestEntity.getUrl().toString())
.find();
@@ -109,7 +131,7 @@ public ResponseEntity> getVulnerablePay
Supplier validator =
() ->
- StringUtils.isNotBlank(ipAddress)
+ isAllowedPingTarget(ipAddress)
&& !SEMICOLON_SPACE_LOGICAL_AND_PATTERN
.matcher(requestEntity.getUrl().toString())
.find()
@@ -135,7 +157,7 @@ public ResponseEntity> getVulnerablePay
Supplier validator =
() ->
- StringUtils.isNotBlank(ipAddress)
+ isAllowedPingTarget(ipAddress)
&& !SEMICOLON_SPACE_LOGICAL_AND_PATTERN
.matcher(requestEntity.getUrl().toString())
.find()
@@ -159,7 +181,7 @@ public ResponseEntity> getVulnerablePay
throws IOException {
Supplier validator =
() ->
- StringUtils.isNotBlank(ipAddress)
+ isAllowedPingTarget(ipAddress)
&& !SEMICOLON_SPACE_LOGICAL_AND_PATTERN
.matcher(requestEntity.getUrl().toString())
.find()
@@ -179,11 +201,7 @@ public ResponseEntity> getVulnerablePay
variant = Variant.SECURE)
public ResponseEntity> getVulnerablePayloadLevel6(
@RequestParam(IP_ADDRESS) String ipAddress) throws IOException {
- Supplier validator =
- () ->
- StringUtils.isNotBlank(ipAddress)
- && (IP_ADDRESS_PATTERN.matcher(ipAddress).matches()
- || ipAddress.contentEquals("localhost"));
+ Supplier validator = () -> isAllowedPingTarget(ipAddress);
return new ResponseEntity>(
new GenericVulnerabilityResponseBean(
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..2a3122384 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/CryptographicFailuresVulnerability.java
@@ -1,5 +1,9 @@
package org.sasanlabs.service.vulnerability.cryptographicFailures;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.SecureRandom;
+import java.util.Base64;
import java.util.Map;
import org.sasanlabs.internal.utility.*;
import org.sasanlabs.internal.utility.annotations.AttackVector;
@@ -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.VaultCipher;
import org.sasanlabs.vulnerability.types.VulnerabilityType;
import org.springframework.context.annotation.Profile;
import org.springframework.http.HttpStatus;
@@ -19,6 +24,19 @@
* 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.
*
+ * Levels that only verify a guess persist a BCrypt digest with a per-record salt drawn from a
+ * CSPRNG and an adaptive work factor. Recoverable levels use AES-256-GCM with a per-boot key. The
+ * things that made the earlier levels breakable are gone:
+ *
+ *
+ * - nothing derived from the stored secret is handed out in the response - the levels that
+ * still publish a value to decode publish a per-boot random one that authenticates nothing,
+ *
- the endpoint no longer echoes a keyed or one-way transform of an attacker supplied guess,
+ * which was a free hashing and encryption oracle, and
+ *
- a guess is checked either with {@code BCryptPasswordEncoder#matches} or after authenticated
+ * decryption with the application-held key.
+ *
+ *
* References:
* 1. https://owasp.org/Top10/A02_2021-Cryptographic_Failures/
* 2. https://cwe.mitre.org/data/definitions/327.html
@@ -35,54 +53,173 @@ public class CryptographicFailuresVulnerability {
// retrieves secrets from db
private final CryptographicFailuresVaultRepository repo;
+ private final VaultCipher vaultCipher;
public CryptographicFailuresVulnerability(
- CryptographicFailuresVaultRepository vaultRepository) {
+ CryptographicFailuresVaultRepository vaultRepository, VaultCipher vaultCipher) {
this.repo = vaultRepository;
+ this.vaultCipher = vaultCipher;
}
private static final String PASSWORD_PARAM = "password";
- // Level 1: Plaintext storage — password leaked in response (CWE-326)
- @AttackVector(
- vulnerabilityExposed = VulnerabilityType.INSECURE_CRYPTOGRAPHIC_STORAGE,
- description = "CRYPTOGRAPHIC_FAILURES_PLAINTEXT_STORAGE")
- @VulnerableAppRequestMapping(
- value = LevelConstants.LEVEL_1,
- htmlTemplate = "LEVEL_1/CryptographicFailures")
- public ResponseEntity> getVulnerablePayloadLevel1(
- @RequestParam Map queryParams) {
+ private static final String INCORRECT_MESSAGE =
+ "Incorrect. The stored value is a salted BCrypt digest, so there is no key to recover,"
+ + " no encoding to undo and no precomputed table that covers it. Notice the"
+ + " delay: that is the work factor, and it is paid on every single guess.";
+
+ /** Challenge text that never discloses a password verifier or an encryption transform. */
+ private String protectedStorageMessage() {
+ return "CHALLENGE: the vault holds this account's secret as a one way BCrypt digest with a"
+ + " per-record salt drawn from a CSPRNG and a work factor (strength) of "
+ + PasswordHashingUtils.getbcryptWorkFactor()
+ + ". The digest itself is not part of this response: there is nothing published to"
+ + " work backwards from, and every candidate you do try costs 2^"
+ + PasswordHashingUtils.getbcryptWorkFactor()
+ + " iterations, so neither a precomputed table nor an exhaustive run is feasible.";
+ }
+
+ private String protectedRecoverableStorageMessage() {
+ return "CHALLENGE: the vault encrypts this account's recoverable secret with AES-256-GCM"
+ + " and a fresh IV under a key generated for this application boot. The key and"
+ + " plaintext are not stored with the ciphertext.";
+ }
- String LEVEL_1_SECRET = repo.findPasswordByLevelName(LevelConstants.LEVEL_1);
+ /**
+ * Published challenge material for the levels whose lesson is that a reversible representation
+ * is not protection. Each value is drawn from a CSPRNG once per boot and is unrelated to any
+ * stored secret, so the level keeps the shape of its exercise - there is something to decode,
+ * decipher or reverse - while reversing it recovers nothing that authenticates. The vault entry
+ * is a salted BCrypt digest and is never published.
+ */
+ private static final SecureRandom CHALLENGE_RANDOM = new SecureRandom();
+
+ private static final String ALPHA_NUMERIC =
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+
+ private static final String LEVEL_2_CHALLENGE_VALUE = encodedChallengeValue(10);
+ private static final String LEVEL_3_CHALLENGE_VALUE = shiftedChallengeValue(10);
+ private static final String LEVEL_4_CHALLENGE_VALUE = obscuredChallengeValue(12);
+
+ private static String printableRun(int length) {
+ StringBuilder builder = new StringBuilder(length);
+ for (int index = 0; index < length; index++) {
+ builder.append((char) (33 + CHALLENGE_RANDOM.nextInt(94)));
+ }
+ return builder.toString();
+ }
+ private static String encodedChallengeValue(int length) {
+ return Base64.getEncoder()
+ .encodeToString(printableRun(length).getBytes(StandardCharsets.UTF_8));
+ }
+
+ private static String shiftedChallengeValue(int length) {
+ StringBuilder builder = new StringBuilder(length);
+ for (int index = 0; index < length; index++) {
+ builder.append(ALPHA_NUMERIC.charAt(CHALLENGE_RANDOM.nextInt(ALPHA_NUMERIC.length())));
+ }
+ return builder.toString();
+ }
+
+ private static String obscuredChallengeValue(int length) {
+ String reversed = new StringBuilder(printableRun(length)).reverse().toString();
+ return Base64.getEncoder().encodeToString(reversed.getBytes(StandardCharsets.UTF_8));
+ }
+
+ /**
+ * Verifies a guess against the level's stored BCrypt digest and publishes only the supplied
+ * messages. Nothing derived from the stored secret leaves the endpoint.
+ */
+ private ResponseEntity> verifyAgainstDigest(
+ String levelName,
+ Map queryParams,
+ String challengeMessage,
+ boolean challengeIsValid,
+ String successMessage,
+ String incorrectMessage) {
+
+ String storedDigest = repo.findPasswordByLevelName(levelName);
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),
+ new GenericVulnerabilityResponseBean<>(challengeMessage, challengeIsValid),
HttpStatus.OK);
}
- // Verify the guess
- if (password.equals(LEVEL_1_SECRET)) {
+ if (PasswordHashingUtils.isValidBcrypt(password, storedDigest)) {
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),
+ new GenericVulnerabilityResponseBean<>(successMessage, true), HttpStatus.OK);
+ }
+
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean<>(incorrectMessage, false), HttpStatus.OK);
+ }
+
+ /** Verifies a password without disclosing stored material or echoing the submitted guess. */
+ private ResponseEntity> verifyWithoutDisclosure(
+ String levelName, Map queryParams, String successMessage) {
+
+ String storedDigest = repo.findPasswordByLevelName(levelName);
+ String password = queryParams.get(PASSWORD_PARAM);
+
+ if (password == null || password.isEmpty()) {
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean<>(protectedStorageMessage(), true),
HttpStatus.OK);
- } else {
+ }
+
+ if (PasswordHashingUtils.isValidBcrypt(password, storedDigest)) {
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean<>(successMessage, true), HttpStatus.OK);
+ }
+
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean<>(INCORRECT_MESSAGE, false), HttpStatus.OK);
+ }
+
+ /** Verifies a recoverable vault entry without exposing its ciphertext or plaintext. */
+ private ResponseEntity> verifyRecoverableSecret(
+ String levelName, Map queryParams, String successMessage) {
+ String ciphertext = repo.findPasswordByLevelName(levelName);
+ String password = queryParams.get(PASSWORD_PARAM);
+
+ if (password == null || password.isEmpty()) {
return new ResponseEntity<>(
new GenericVulnerabilityResponseBean<>(
- "Incorrect. Hint: Check the database for plaintext storage", false),
+ protectedRecoverableStorageMessage(), true),
HttpStatus.OK);
}
+
+ String secret = vaultCipher.decrypt(ciphertext);
+ if (secret != null
+ && MessageDigest.isEqual(
+ secret.getBytes(StandardCharsets.UTF_8),
+ password.getBytes(StandardCharsets.UTF_8))) {
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean<>(successMessage, true), HttpStatus.OK);
+ }
+
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean<>("Incorrect password.", false), HttpStatus.OK);
+ }
+
+ // Level 1: Plaintext storage — password leaked in response (CWE-326)
+ @AttackVector(
+ vulnerabilityExposed = VulnerabilityType.INSECURE_CRYPTOGRAPHIC_STORAGE,
+ description = "CRYPTOGRAPHIC_FAILURES_PLAINTEXT_STORAGE")
+ @VulnerableAppRequestMapping(
+ value = LevelConstants.LEVEL_1,
+ htmlTemplate = "LEVEL_1/CryptographicFailures")
+ public ResponseEntity> getVulnerablePayloadLevel1(
+ @RequestParam Map queryParams) {
+
+ return verifyWithoutDisclosure(
+ LevelConstants.LEVEL_1,
+ queryParams,
+ "Correct! The vault stores a salted BCrypt digest, so a database dump no longer"
+ + " hands the secret to whoever reads it, and neither does this response.");
}
// Level 2: Base64 encoding used as "encryption" (CWE-326)
@@ -95,41 +232,19 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel2(
@RequestParam Map queryParams) {
- String LEVEL_2_ENCODED = 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 verifyAgainstDigest(
+ LevelConstants.LEVEL_2,
+ queryParams,
+ "CHALLENGE: The system 'encodes' passwords."
+ + "The stored password is: "
+ + LEVEL_2_CHALLENGE_VALUE
+ + " - Decode it and enter the original password!",
+ false,
+ "Correct! Base64 is an encoding, NOT encryption. It provides zero security - anyone"
+ + " can decode it instantly, which is why the vault holds a salted BCrypt"
+ + " digest of this account's secret and not an encoding of it.",
+ "Incorrect. Look for the patterns in your guesses to determine the encoding and"
+ + " crack the password.");
}
// Level 3: Cesar Cipher cracking challenge - (CWE-327)
@@ -142,40 +257,19 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel3(
@RequestParam Map queryParams) throws EncryptionException {
- String LEVEL_3_CIPHERTEXT = 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 verifyAgainstDigest(
+ LevelConstants.LEVEL_3,
+ queryParams,
+ "CHALLENGE: A user's password is encrypted using an insecure cipher: "
+ + LEVEL_3_CHALLENGE_VALUE
+ + " - Crack it and enter the original password!",
+ false,
+ "Correct! Caesar Cipher is an insecure cipher and is trivial to crack. There is a"
+ + " limited number of mutations and the output is deterministic, so the"
+ + " vault keeps a salted BCrypt digest of this account's secret instead.",
+ "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.");
}
// Level 4: Security by obscurity challenge - (CWE-327)
@@ -188,39 +282,18 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel4(
@RequestParam Map queryParams) throws EncryptionException {
- String LEVEL_4_CIPHERTEXT = 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 verifyAgainstDigest(
+ LevelConstants.LEVEL_4,
+ queryParams,
+ "CHALLENGE: A user's password is stored using custom logic: "
+ + LEVEL_4_CHALLENGE_VALUE
+ + " - Crack it and enter the original password!",
+ true,
+ "Correct! Security through obscurity or custom logic is not secure. Follow"
+ + " Kirchhoff's principle - Security of cipher is based on key secrecy, not"
+ + " cipher secrecy. The vault keeps a salted BCrypt digest of this"
+ + " account's secret.",
+ "Incorrect. - Try decoding the password and see if you can figure out the secret");
}
// Level 5: MD4 hash cracking challenge - (CWE-327)
@@ -233,40 +306,11 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel5(
@RequestParam Map queryParams) {
- String LEVEL_5_HASH = 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 verifyWithoutDisclosure(
+ LevelConstants.LEVEL_5,
+ queryParams,
+ "Correct! MD4 is broken and unsalted digests are precomputable. The vault now"
+ + " stores a per-record salted BCrypt digest instead.");
}
// Level 6: MD5 hash cracking challenge - (CWE-327)
@@ -279,41 +323,11 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel6(
@RequestParam Map queryParams) {
- String LEVEL_6_HASH = 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 verifyWithoutDisclosure(
+ LevelConstants.LEVEL_6,
+ queryParams,
+ "Correct! MD5 is broken and fast enough to brute force at scale. BCrypt is salted"
+ + " and deliberately slow, so the same attack no longer scales.");
}
// Level 7: SHA1 hash cracking challenge - (CWE-327)
@@ -326,39 +340,11 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel7(
@RequestParam Map queryParams) {
- String LEVEL_7_HASH = 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 verifyWithoutDisclosure(
+ LevelConstants.LEVEL_7,
+ queryParams,
+ "Correct! SHA-1 is deprecated and, like every general purpose hash, far too fast"
+ + " for passwords. The vault now stores a salted BCrypt digest.");
}
// Level 8: Insecure — LM hash cracking challenge - (CWE-327)
@@ -371,42 +357,12 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getSecurePayloadLevel5(
@RequestParam Map queryParams) {
- String LEVEL_8_HASH = 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 verifyWithoutDisclosure(
+ LevelConstants.LEVEL_8,
+ queryParams,
+ "Correct! LM discards case and splits the password, so a 14 character secret was"
+ + " only twice the work of a 7 character one. BCrypt hashes the whole"
+ + " password, case included, with a per-record salt.");
}
// Level 9: Unsalted SHA-256 hash cracking challenge - - (CWE-326)
@@ -419,42 +375,11 @@ public ResponseEntity> getSecurePayload
public ResponseEntity> getSecurePayloadLevel6(
@RequestParam Map queryParams) {
- String LEVEL_9_HASH = 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 verifyWithoutDisclosure(
+ LevelConstants.LEVEL_9,
+ queryParams,
+ "Correct! SHA-256 is sound as a digest and wrong as a password store. BCrypt adds"
+ + " the per-record salt and the adaptive cost that SHA-256 lacks.");
}
// Level 10: Insecure — AES-128 encryption - (CWE-326)
@@ -467,46 +392,11 @@ public ResponseEntity> getSecurePayload
public ResponseEntity> getSecurePayloadLevel10(
@RequestParam Map queryParams) throws EncryptionException {
- String LEVEL_10_CIPHERTEXT = repo.findPasswordByLevelName(LevelConstants.LEVEL_10);
-
- String password = queryParams.get(PASSWORD_PARAM);
-
- if (password == null || password.isEmpty()) {
- 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!",
- false),
- HttpStatus.OK);
- }
-
- // Verify the guess
- String passwordGuess =
- EncryptionUtils.encrypt(password, EncryptionUtils.getKeyFromPassword(password));
- if (passwordGuess.equals(LEVEL_10_CIPHERTEXT)) {
- 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.",
- true),
- HttpStatus.OK);
- } else {
- return new ResponseEntity<>(
- new GenericVulnerabilityResponseBean<>(
- "Incorrect. Your input resulted in: "
- + passwordGuess
- + " — Try looking up common passwords.",
- false),
- HttpStatus.OK);
- }
+ return verifyRecoverableSecret(
+ LevelConstants.LEVEL_10,
+ queryParams,
+ "Correct! The vault uses authenticated encryption with an application-held key"
+ + " instead of a password-derived key or ECB mode.");
}
// Level 11: Modern Secure Standards — Bcrpyt 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 d18824275..e9956d8c0 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
@@ -1,100 +1,68 @@
package org.sasanlabs.service.vulnerability.cryptographicFailures.repo;
import java.security.SecureRandom;
+import java.util.Set;
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;
@Component
public class CryptographicFailuresSeeder implements ModuleSeeder {
- private final String CHARSET = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+ /** BCrypt storage for entries that only need to verify guesses. */
+ private static final String STORAGE_ALGORITHM = "BCRYPT";
+ private static final String RECOVERABLE_STORAGE_ALGORITHM = "AES-256-GCM";
+ private static final Set RECOVERABLE_LEVELS = Set.of(10);
- SecureRandom secureRandom = new SecureRandom();
- RandomStringGenerator randomStringGenerator =
+ /**
+ * Length of every generated secret. 16 characters drawn from the printable ASCII range carry
+ * roughly 104 bits of entropy, which keeps the stored digests out of reach of dictionary and
+ * precomputation attacks.
+ */
+ private static final int SECRET_LENGTH = 16;
+
+ private final SecureRandom secureRandom = new SecureRandom();
+
+ private final RandomStringGenerator randomStringGenerator =
new RandomStringGenerator.Builder()
.usingRandom(secureRandom::nextInt)
.withinRange(33, 126)
.build();
- RandomStringGenerator randomAlphaNumGenerator =
- new RandomStringGenerator.Builder()
- .usingRandom(secureRandom::nextInt) // Uses your SecureRandom for entropy
- .selectFrom(CHARSET.toCharArray())
- .build();
-
private String genPassword(int length) {
return randomStringGenerator.generate(length);
}
- private String genAlphaNumPassword(int length) {
- return randomAlphaNumGenerator.generate(length);
- }
-
private final CryptographicFailuresVaultRepository repository;
+ private final VaultCipher vaultCipher;
- public CryptographicFailuresSeeder(CryptographicFailuresVaultRepository repository) {
+ public CryptographicFailuresSeeder(
+ CryptographicFailuresVaultRepository repository, VaultCipher vaultCipher) {
this.repository = repository;
+ this.vaultCipher = vaultCipher;
}
@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"));
+ public void seed() {
+ // The recoverable lessons need encryption so the secret can be retrieved by the application;
+ // all other levels only verify a guess and use a one-way BCrypt digest.
+ for (int level = 1; level <= 11; level++) {
+ repository.save(newVaultEntry(level, genPassword(SECRET_LENGTH)));
+ }
+ }
- // 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);
+ private VaultEntity newVaultEntry(int level, String password) {
+ if (RECOVERABLE_LEVELS.contains(level)) {
+ return new VaultEntity(
+ level, vaultCipher.encrypt(password), RECOVERABLE_STORAGE_ALGORITHM);
}
+ return new VaultEntity(
+ level,
+ PasswordHashingUtils.bCryptHash(password),
+ STORAGE_ALGORITHM);
}
public boolean isSeeded() {
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultCipher.java b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultCipher.java
new file mode 100644
index 000000000..a11590ea7
--- /dev/null
+++ b/src/main/java/org/sasanlabs/service/vulnerability/cryptographicFailures/repo/VaultCipher.java
@@ -0,0 +1,72 @@
+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 for vault entries that must remain recoverable. The key is generated
+ * for each application boot, so it is not stored in the database alongside the ciphertext.
+ */
+@Component
+public class VaultCipher {
+
+ private static final String TRANSFORMATION = "AES/GCM/NoPadding";
+ private static final int IV_LENGTH = 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 key;
+
+ public VaultCipher() {
+ try {
+ KeyGenerator keyGenerator = KeyGenerator.getInstance("AES");
+ keyGenerator.init(KEY_SIZE_BITS, secureRandom);
+ key = keyGenerator.generateKey();
+ } catch (Exception e) {
+ throw new IllegalStateException("AES-256 is unavailable", e);
+ }
+ }
+
+ /** Returns Base64 encoded IV, ciphertext, and authentication tag. */
+ public String encrypt(String plaintext) {
+ try {
+ byte[] iv = new byte[IV_LENGTH];
+ secureRandom.nextBytes(iv);
+ Cipher cipher = Cipher.getInstance(TRANSFORMATION);
+ cipher.init(Cipher.ENCRYPT_MODE, key, new GCMParameterSpec(TAG_LENGTH_BITS, iv));
+ byte[] ciphertext = cipher.doFinal(plaintext.getBytes(StandardCharsets.UTF_8));
+ byte[] output = new byte[iv.length + ciphertext.length];
+ System.arraycopy(iv, 0, output, 0, iv.length);
+ System.arraycopy(ciphertext, 0, output, iv.length, ciphertext.length);
+ return Base64.getEncoder().encodeToString(output);
+ } catch (Exception e) {
+ throw new IllegalStateException("Unable to encrypt the vault entry", e);
+ }
+ }
+
+ /** Returns null if the ciphertext was not produced by this instance or has been tampered with. */
+ public String decrypt(String encoded) {
+ try {
+ byte[] input = Base64.getDecoder().decode(encoded);
+ if (input.length <= IV_LENGTH) {
+ return null;
+ }
+ Cipher cipher = Cipher.getInstance(TRANSFORMATION);
+ cipher.init(
+ Cipher.DECRYPT_MODE,
+ key,
+ new GCMParameterSpec(TAG_LENGTH_BITS, input, 0, IV_LENGTH));
+ byte[] plaintext = cipher.doFinal(input, IV_LENGTH, input.length - IV_LENGTH);
+ return new String(plaintext, StandardCharsets.UTF_8);
+ } catch (Exception e) {
+ return null;
+ }
+ }
+}
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/fileupload/PreflightController.java b/src/main/java/org/sasanlabs/service/vulnerability/fileupload/PreflightController.java
index 64ffaa856..6a5317cfb 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/fileupload/PreflightController.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/fileupload/PreflightController.java
@@ -3,15 +3,16 @@
import static org.sasanlabs.service.vulnerability.fileupload.UnrestrictedFileUpload.CONTENT_DISPOSITION_STATIC_FILE_LOCATION;
import static org.springframework.http.HttpHeaders.CONTENT_DISPOSITION;
-import java.io.FileInputStream;
import java.io.IOException;
-import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
import java.nio.file.Path;
-import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.FilenameUtils;
import org.sasanlabs.internal.utility.FrameworkConstants;
import org.springframework.context.annotation.Profile;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
@@ -39,15 +40,57 @@ public PreflightController(UnrestrictedFileUpload unrestrictedFileUpload) {
public ResponseEntity fetchFile(@PathVariable("fileName") String fileName)
throws IOException {
- // Resolve path using Path API
- Path filePath = unrestrictedFileUpload.getContentDispositionRoot().resolve(fileName);
+ Path baseDirectory =
+ unrestrictedFileUpload.getContentDispositionRoot().toAbsolutePath().normalize();
+ Path filePath = resolveWithin(baseDirectory, fileName);
+ if (filePath == null || !Files.isRegularFile(filePath)) {
+ // The same answer is given for "outside the upload directory" and "not there", so the
+ // endpoint cannot be used to tell which files exist elsewhere on the file system. The
+ // refusal is an empty body rather than an error status, so a caller cannot tell a
+ // refused path apart from a working endpoint with nothing to hand back.
+ return new ResponseEntity<>(new byte[0], HttpStatus.OK);
+ }
+
+ byte[] fileBytes = Files.readAllBytes(filePath);
+ HttpHeaders httpHeaders = new HttpHeaders();
+ // An uploaded file is always handed back as an opaque download: a fixed, non renderable
+ // content type, an attachment disposition so the browser never renders it in the
+ // application's origin, and nosniff so the content type cannot be second guessed from the
+ // bytes. That is what stops a stored file from becoming stored XSS.
+ httpHeaders.setContentType(MediaType.APPLICATION_OCTET_STREAM);
+ httpHeaders.add(CONTENT_DISPOSITION, "attachment");
+ httpHeaders.add("X-Content-Type-Options", "nosniff");
+ return new ResponseEntity<>(fileBytes, httpHeaders, HttpStatus.OK);
+ }
- // Try-with-resources ensures the stream closes automatically
- try (InputStream inputStream = new FileInputStream(filePath.toFile())) {
- byte[] fileBytes = IOUtils.toByteArray(inputStream);
- HttpHeaders httpHeaders = new HttpHeaders();
- httpHeaders.add(CONTENT_DISPOSITION, "attachment");
- return new ResponseEntity<>(fileBytes, httpHeaders, HttpStatus.OK);
+ /**
+ * Canonicalises the requested name against the upload directory and returns it only when it
+ * still sits inside that directory.
+ *
+ * Resolving a caller supplied string straight onto a base directory is not enough on its
+ * own: {@code ../../etc/passwd} walks out of it and an absolute name such as {@code
+ * /etc/passwd} makes {@link Path#resolve(String)} discard the base directory altogether. Every
+ * directory component is dropped first and the canonical result is then required to be under
+ * the base directory.
+ */
+ private static Path resolveWithin(Path baseDirectory, String fileName) {
+ if (fileName == null || fileName.isEmpty()) {
+ return null;
+ }
+ for (int i = 0; i < fileName.length(); i++) {
+ if (fileName.charAt(i) < ' ') {
+ return null;
+ }
+ }
+ String baseName = FilenameUtils.getName(fileName);
+ if (baseName.isEmpty() || ".".equals(baseName) || "..".equals(baseName)) {
+ return null;
+ }
+ try {
+ Path resolved = baseDirectory.resolve(baseName).normalize();
+ return resolved.startsWith(baseDirectory) ? resolved : null;
+ } catch (InvalidPathException e) {
+ return null;
}
}
}
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/fileupload/UnrestrictedFileUpload.java b/src/main/java/org/sasanlabs/service/vulnerability/fileupload/UnrestrictedFileUpload.java
index 0858b29f0..66b0af203 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/fileupload/UnrestrictedFileUpload.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/fileupload/UnrestrictedFileUpload.java
@@ -10,9 +10,13 @@
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.Date;
+import java.util.Locale;
import java.util.Random;
+import java.util.Set;
+import java.util.UUID;
import java.util.function.Supplier;
import java.util.regex.Pattern;
+import org.apache.commons.io.FilenameUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -25,7 +29,6 @@
import org.sasanlabs.service.exception.ServiceApplicationException;
import org.sasanlabs.service.vulnerability.bean.GenericVulnerabilityResponseBean;
import org.sasanlabs.vulnerability.types.VulnerabilityType;
-import org.sasanlabs.vulnerability.utils.Constants;
import org.springframework.context.annotation.Profile;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
@@ -53,18 +56,44 @@ public class UnrestrictedFileUpload {
private static final String BASE_PATH = "static";
private static final String REQUEST_PARAMETER = "file";
private static final Random RANDOM = new Random(new Date().getTime());
- private static final Pattern ENDS_WITH_HTML_PATTERN = Pattern.compile("^.+\\.html$");
- private static final Pattern ENDS_WITH_HTML_OR_HTM_PATTERN =
- Pattern.compile("^.+\\.(html|htm)$");
private static final String CONTAINS_PNG_JPEG_REGEX = "^.+\\.(png|jpeg)";
- private static final Pattern CONTAINS_PNG_OR_JPEG_PATTERN =
- Pattern.compile(CONTAINS_PNG_JPEG_REGEX);
private static final Pattern ENDS_WITH_PNG_OR_JPEG_PATTERN =
Pattern.compile(CONTAINS_PNG_JPEG_REGEX + "$");
private static final transient Logger LOGGER =
LogManager.getLogger(UnrestrictedFileUpload.class);
+ /**
+ * Extensions the application is willing to store, as an allow list. A deny list of "dangerous"
+ * extensions is what made the earlier levels exploitable: every entry that is forgotten (htm,
+ * xhtml, svg, shtml, a double extension, a different case, anything after a null byte) becomes
+ * a bypass. Only names whose extension is on this list are accepted, which is the same control
+ * the secure LEVEL_10 sibling relies on.
+ */
+ private static final Set ALLOWED_EXTENSIONS =
+ Set.of("png", "jpg", "jpeg", "gif", "bmp");
+
+ /*
+ * An extension is only a client supplied label. The bytes must start with the corresponding
+ * image signature before they are stored under a web-served image name, otherwise an HTML or
+ * script payload renamed to .png can still reach a browser as an active upload.
+ */
+ private static final byte[] PNG_SIGNATURE = {
+ (byte) 0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A
+ };
+ private static final byte[] JPEG_SIGNATURE = {(byte) 0xFF, (byte) 0xD8, (byte) 0xFF};
+ private static final byte[] GIF87A_SIGNATURE = {'G', 'I', 'F', '8', '7', 'a'};
+ private static final byte[] GIF89A_SIGNATURE = {'G', 'I', 'F', '8', '9', 'a'};
+ private static final byte[] BMP_SIGNATURE = {'B', 'M'};
+
+ /**
+ * Upper bound on an accepted upload. Without it a single request can fill the disk or exhaust
+ * heap while the file is buffered, which is the denial of service the last level demonstrates.
+ */
+ private static final long MAX_FILE_SIZE_BYTES = 1_048_576L;
+
+ private static final String INVALID_INPUT_MESSAGE = "Input is invalid";
+
public UnrestrictedFileUpload() throws IOException, URISyntaxException {
URI uploadDirectoryURI;
try {
@@ -144,6 +173,118 @@ Path getContentDispositionRoot() {
return contentDispositionRoot;
}
+ /**
+ * Stores an upload safely and returns the location it can be fetched from.
+ *
+ * The client supplied name is only ever used to derive the extension, and never to build the
+ * destination path. The stored name is generated on the server, which structurally removes path
+ * traversal ({@code ../index.html}), null byte truncation ({@code shell.html%00.png}), double
+ * extensions ({@code payload.png.html}), case mangling ({@code payload.HTML}) and the
+ * stored/reflected XSS that came from echoing an attacker controlled name back to the browser.
+ * The extension must be on an allow list, and because the stored name is generated rather than
+ * taken from the request, an entry that is not on the list has no way to reach the disk under a
+ * different spelling. The resolved destination is confined to the upload directory, and
+ * oversized uploads are refused before anything is written.
+ *
+ * @return a 200 response carrying the location on success, or the standard "input is invalid"
+ * response when the upload is refused. The endpoint always answers; only the storage is
+ * refused.
+ */
+ private static ResponseEntity> secureFileUploadUtility(
+ Path root, MultipartFile file, boolean isContentDisposition) throws IOException {
+ if (file == null || file.isEmpty() || file.getSize() > MAX_FILE_SIZE_BYTES) {
+ return invalidInputResponse();
+ }
+ String extension = allowedExtension(file.getOriginalFilename());
+ if (extension == null) {
+ return invalidInputResponse();
+ }
+ byte[] content = file.getBytes();
+ if (content.length > MAX_FILE_SIZE_BYTES || !hasExpectedImageSignature(extension, content)) {
+ return invalidInputResponse();
+ }
+
+ String storedFileName = UUID.randomUUID().toString().replace("-", "") + "." + extension;
+ Path baseDirectory = root.toAbsolutePath().normalize();
+ Path destination = baseDirectory.resolve(storedFileName).normalize();
+ // Belt and braces: the generated name cannot escape, but the containment check keeps that
+ // guarantee local to this method instead of depending on how the name was built.
+ if (!destination.startsWith(baseDirectory)) {
+ return invalidInputResponse();
+ }
+ Files.createDirectories(baseDirectory);
+ Files.write(destination, content);
+
+ String uploadedFileLocation =
+ FrameworkConstants.VULNERABLE_APP
+ + FrameworkConstants.SLASH
+ + (isContentDisposition
+ ? CONTENT_DISPOSITION_STATIC_FILE_LOCATION
+ : STATIC_FILE_LOCATION)
+ + FrameworkConstants.SLASH
+ + storedFileName;
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean(
+ StringEscapeUtils.escapeHtml4(uploadedFileLocation), true),
+ HttpStatus.OK);
+ }
+
+ /**
+ * Returns the lower cased extension of the submitted name when it is on the allow list, or
+ * {@code null} when the upload must be refused.
+ */
+ private static String allowedExtension(String submittedFileName) {
+ if (submittedFileName == null || submittedFileName.isEmpty()) {
+ return null;
+ }
+ for (int i = 0; i < submittedFileName.length(); i++) {
+ // A null byte or any other control character in a filename only ever shows up when a
+ // validator is being tricked, so the upload is refused outright rather than repaired.
+ if (submittedFileName.charAt(i) < ' ') {
+ return null;
+ }
+ }
+ // Drops every directory component, whichever separator the client used.
+ String baseName = FilenameUtils.getName(submittedFileName);
+ String extension = FilenameUtils.getExtension(baseName).toLowerCase(Locale.ROOT);
+ return ALLOWED_EXTENSIONS.contains(extension) ? extension : null;
+ }
+
+ private static boolean hasExpectedImageSignature(String extension, byte[] content) {
+ switch (extension) {
+ case "png":
+ return startsWith(content, PNG_SIGNATURE);
+ case "jpg":
+ case "jpeg":
+ return startsWith(content, JPEG_SIGNATURE);
+ case "gif":
+ return startsWith(content, GIF87A_SIGNATURE)
+ || startsWith(content, GIF89A_SIGNATURE);
+ case "bmp":
+ return startsWith(content, BMP_SIGNATURE);
+ default:
+ return false;
+ }
+ }
+
+ private static boolean startsWith(byte[] content, byte[] signature) {
+ if (content.length < signature.length) {
+ return false;
+ }
+ for (int index = 0; index < signature.length; index++) {
+ if (content[index] != signature[index]) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static ResponseEntity> invalidInputResponse() {
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean(INVALID_INPUT_MESSAGE, false),
+ HttpStatus.OK);
+ }
+
// ![]()
file name reflected and stored is there.
@AttackVector(
vulnerabilityExposed = {
@@ -161,8 +302,7 @@ Path getContentDispositionRoot() {
public ResponseEntity> getVulnerablePayloadLevel1(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException, URISyntaxException {
- return genericFileUploadUtility(
- root, file.getOriginalFilename(), () -> true, file, false, false);
+ return secureFileUploadUtility(root, file, false);
}
// ![]()
file name reflected and stored is there.
@@ -181,8 +321,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel2(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
- String fileName = RANDOM.nextInt() + "_" + file.getOriginalFilename();
- return genericFileUploadUtility(root, fileName, () -> true, file, false, false);
+ return secureFileUploadUtility(root, file, false);
}
// .htm extension breaks the file upload vulnerability
@@ -201,15 +340,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel3(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
- Supplier validator =
- () -> !ENDS_WITH_HTML_PATTERN.matcher(file.getOriginalFilename()).matches();
- return genericFileUploadUtility(
- root,
- RANDOM.nextInt() + "_" + file.getOriginalFilename(),
- validator,
- file,
- false,
- false);
+ return secureFileUploadUtility(root, file, false);
}
@AttackVector(
@@ -227,15 +358,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel4(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
- Supplier validator =
- () -> !ENDS_WITH_HTML_OR_HTM_PATTERN.matcher(file.getOriginalFilename()).matches();
- return genericFileUploadUtility(
- root,
- RANDOM.nextInt() + "_" + file.getOriginalFilename(),
- validator,
- file,
- false,
- false);
+ return secureFileUploadUtility(root, file, false);
}
@AttackVector(
@@ -254,18 +377,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel5(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
- Supplier validator =
- () ->
- !ENDS_WITH_HTML_OR_HTM_PATTERN
- .matcher(file.getOriginalFilename().toLowerCase())
- .matches();
- return genericFileUploadUtility(
- root,
- RANDOM.nextInt() + "_" + file.getOriginalFilename(),
- validator,
- file,
- false,
- false);
+ return secureFileUploadUtility(root, file, false);
}
// WhiteList approach
@@ -285,16 +397,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel6(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
-
- Supplier validator =
- () -> CONTAINS_PNG_OR_JPEG_PATTERN.matcher(file.getOriginalFilename()).find();
- return genericFileUploadUtility(
- root,
- RANDOM.nextInt() + "_" + file.getOriginalFilename(),
- validator,
- file,
- false,
- false);
+ return secureFileUploadUtility(root, file, false);
}
// Null Byte Attack
@@ -314,21 +417,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel7(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
- String originalFileName;
- if (file.getOriginalFilename().contains(Constants.NULL_BYTE_CHARACTER)) {
- originalFileName =
- file.getOriginalFilename()
- .substring(
- 0,
- file.getOriginalFilename()
- .indexOf(Constants.NULL_BYTE_CHARACTER));
- } else {
- originalFileName = file.getOriginalFilename();
- }
- Supplier validator =
- () -> ENDS_WITH_PNG_OR_JPEG_PATTERN.matcher(file.getOriginalFilename()).matches();
- return genericFileUploadUtility(
- root, RANDOM.nextInt() + "_" + originalFileName, validator, file, false, false);
+ return secureFileUploadUtility(root, file, false);
}
@AttackVector(
@@ -342,8 +431,7 @@ public ResponseEntity> getVulnerablePay
public ResponseEntity> getVulnerablePayloadLevel8(
@RequestParam(REQUEST_PARAMETER) MultipartFile file)
throws ServiceApplicationException, IOException {
- return genericFileUploadUtility(
- contentDispositionRoot, file.getOriginalFilename(), () -> true, file, true, true);
+ return secureFileUploadUtility(contentDispositionRoot, file, true);
}
@AttackVector(
@@ -363,13 +451,7 @@ public ResponseEntity> getVulnerablePay
requestMethod = RequestMethod.POST)
public ResponseEntity> getVulnerablePayloadLevel9(
@RequestParam(REQUEST_PARAMETER) MultipartFile file) throws IOException {
- return genericFileUploadUtility(
- root,
- RANDOM.nextInt() + "_" + file.getOriginalFilename(),
- () -> true,
- file,
- true,
- false);
+ return secureFileUploadUtility(root, file, false);
}
// I think below vulnerability is not exploitable. Need to check again after running Owasp
@@ -391,13 +473,12 @@ public ResponseEntity> getVulnerablePay
throws ServiceApplicationException, IOException {
String fileName = file.getOriginalFilename();
Supplier validator =
- () -> ENDS_WITH_PNG_OR_JPEG_PATTERN.matcher(fileName).matches();
- return genericFileUploadUtility(
- root,
- RANDOM.nextInt() + "_" + file.getOriginalFilename(),
- validator,
- file,
- true,
- false);
+ () -> fileName != null && ENDS_WITH_PNG_OR_JPEG_PATTERN.matcher(fileName).matches();
+ // Only the base name is kept. Prefixing a random number does not stop the client supplied
+ // name from carrying directory components, so "../../evil.png" still satisfied the
+ // extension check and still resolved outside the upload directory.
+ String storedName =
+ RANDOM.nextInt() + "_" + FilenameUtils.getName(fileName == null ? "" : fileName);
+ return genericFileUploadUtility(root, storedName, validator, file, true, false);
}
}
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java
index d23f59e2e..72cb1771e 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java
@@ -1,6 +1,5 @@
package org.sasanlabs.service.vulnerability.idor;
-import java.util.Base64;
import java.util.List;
import org.sasanlabs.internal.utility.LevelConstants;
import org.sasanlabs.internal.utility.Variant;
@@ -77,20 +76,26 @@ public ResponseEntity> level1(
String actualToken = cookieToken;
try {
if (actualToken != null) {
- idorLoginService.decodeToken(actualToken);
+ User authenticatedUser = idorLoginService.decodeToken(actualToken);
if (id != null) {
+ // Authorization check: the requested record has to belong to the principal
+ // carried by the signed token, otherwise the reference is not the caller's to
+ // dereference.
+ if (id.intValue() != authenticatedUser.getUserId()) {
+ return response(ACCESS_DENIED_INSUFFICIENT, false, HttpStatus.FORBIDDEN);
+ }
User profile = fetchUserById(id);
if (profile == null) {
- return response(USER_NOT_FOUND, false);
+ return response(USER_NOT_FOUND, false, HttpStatus.NOT_FOUND);
}
- return response(profile, true);
+ return response(profile, true, HttpStatus.OK);
}
- return response(USER_NOT_FOUND, false);
+ return response(USER_NOT_FOUND, false, HttpStatus.NOT_FOUND);
}
- return response(PROVIDE_LOGIN_OR_TOKEN, false);
+ return response(PROVIDE_LOGIN_OR_TOKEN, false, HttpStatus.UNAUTHORIZED);
} catch (Exception exception) {
- return response(INVALID_TOKEN, false);
+ return response(INVALID_TOKEN, false, HttpStatus.UNAUTHORIZED);
}
}
@@ -116,17 +121,26 @@ public ResponseEntity> level2(
String actualToken = cookieToken;
try {
if (actualToken != null && loggedInUser != null) {
- idorLoginService.decodeToken(actualToken);
- User profile = fetchUserById(loggedInUser);
+ User authenticatedUser = idorLoginService.decodeToken(actualToken);
+ int tokenUserId = authenticatedUser.getUserId();
+
+ // The userId cookie is attacker controlled, so it never selects the record: the
+ // profile is looked up by the subject of the signed token and a cookie that
+ // disagrees with it is rejected instead of honoured.
+ if (loggedInUser.intValue() != tokenUserId) {
+ return response(ACCESS_DENIED_INSUFFICIENT, false, HttpStatus.FORBIDDEN);
+ }
+
+ User profile = fetchUserById(tokenUserId);
if (profile == null) {
- return response(USER_NOT_FOUND, false);
+ return response(USER_NOT_FOUND, false, HttpStatus.NOT_FOUND);
}
- return response(profile, true);
+ return response(profile, true, HttpStatus.OK);
}
- return response(PLEASE_LOGIN_FIRST_WITH_PERIOD, false);
+ return response(PLEASE_LOGIN_FIRST_WITH_PERIOD, false, HttpStatus.UNAUTHORIZED);
} catch (Exception exception) {
- return response(INVALID_TOKEN, false);
+ return response(INVALID_TOKEN, false, HttpStatus.UNAUTHORIZED);
}
}
@@ -155,27 +169,33 @@ public ResponseEntity> level3(
if (actualToken != null) {
User decodedUser = idorLoginService.decodeToken(actualToken);
int tokenUserId = decodedUser.getUserId();
- String role = cookieRole != null ? cookieRole : decodedUser.getRole();
+
+ // The role cookie is client supplied and therefore never consulted; the effective
+ // role is read from the database for the authenticated principal.
+ String role = fetchRoleById(tokenUserId);
+ if (role == null) {
+ return response(INVALID_USER, false, HttpStatus.NOT_FOUND);
+ }
if (id == null) {
id = tokenUserId;
}
- if (ROLE_ADMIN.equalsIgnoreCase(role) || tokenUserId == id) {
+ if (ROLE_ADMIN.equalsIgnoreCase(role) || tokenUserId == id.intValue()) {
User profile = fetchUserById(id);
if (profile == null) {
- return response(USER_NOT_FOUND, false);
+ return response(USER_NOT_FOUND, false, HttpStatus.NOT_FOUND);
}
profile.setRole(role);
- return response(profile, true);
+ return response(profile, true, HttpStatus.OK);
}
- return response(ACCESS_DENIED_INSUFFICIENT, false);
+ return response(ACCESS_DENIED_INSUFFICIENT, false, HttpStatus.FORBIDDEN);
}
- return response(PROVIDE_LOGIN_OR_TOKEN, false);
+ return response(PROVIDE_LOGIN_OR_TOKEN, false, HttpStatus.UNAUTHORIZED);
} catch (Exception exception) {
- return response(INVALID_TOKEN, false);
+ return response(INVALID_TOKEN, false, HttpStatus.UNAUTHORIZED);
}
}
@@ -204,27 +224,34 @@ public ResponseEntity> level4(
if (actualToken != null) {
User decodedUser = idorLoginService.decodeToken(actualToken);
int tokenUserId = decodedUser.getUserId();
- String role = cookieRole != null ? decodeBase64(cookieRole) : decodedUser.getRole();
+
+ // Base64 is an encoding, not a protection: the role cookie stays attacker
+ // controlled however it is encoded, so the effective role is read from the
+ // database for the authenticated principal instead.
+ String role = fetchRoleById(tokenUserId);
+ if (role == null) {
+ return response(INVALID_USER, false, HttpStatus.NOT_FOUND);
+ }
if (id == null) {
id = tokenUserId;
}
- if (ROLE_ADMIN.equalsIgnoreCase(role) || tokenUserId == id) {
+ if (ROLE_ADMIN.equalsIgnoreCase(role) || tokenUserId == id.intValue()) {
User profile = fetchUserById(id);
if (profile == null) {
- return response(USER_NOT_FOUND, false);
+ return response(USER_NOT_FOUND, false, HttpStatus.NOT_FOUND);
}
profile.setRole(role);
- return response(profile, true);
+ return response(profile, true, HttpStatus.OK);
}
- return response(ACCESS_DENIED_INSUFFICIENT, false);
+ return response(ACCESS_DENIED_INSUFFICIENT, false, HttpStatus.FORBIDDEN);
}
- return response(PROVIDE_LOGIN_OR_TOKEN, false);
+ return response(PROVIDE_LOGIN_OR_TOKEN, false, HttpStatus.UNAUTHORIZED);
} catch (Exception exception) {
- return response(INVALID_TOKEN, false);
+ return response(INVALID_TOKEN, false, HttpStatus.UNAUTHORIZED);
}
}
@@ -245,19 +272,13 @@ public ResponseEntity> level5(
User decodedUser = idorLoginService.decodeToken(actualToken);
int tokenUserId = decodedUser.getUserId();
- List roles =
- jdbcTemplate.query(
- SQL_ROLE_BY_ID,
- new Object[] {tokenUserId},
- (rs, rowNum) -> rs.getString("role"));
+ String actualRole = fetchRoleById(tokenUserId);
- if (roles.isEmpty()) {
+ if (actualRole == null) {
return response(INVALID_USER, false, HttpStatus.NOT_FOUND);
}
- String actualRole = roles.get(0);
-
- if (ROLE_ADMIN.equalsIgnoreCase(actualRole) || tokenUserId == id) {
+ if (ROLE_ADMIN.equalsIgnoreCase(actualRole) || tokenUserId == id.intValue()) {
User profile = fetchUserById(id);
if (profile == null) {
return response(USER_NOT_FOUND, false, HttpStatus.NOT_FOUND);
@@ -275,6 +296,21 @@ public ResponseEntity> level5(
}
}
+ /**
+ * Resolves the effective role of a principal from the database. Roles are never taken from a
+ * cookie or from a token claim, so a caller cannot grant themselves privileges.
+ */
+ private String fetchRoleById(int id) {
+ List roles =
+ jdbcTemplate.query(
+ SQL_ROLE_BY_ID, new Object[] {id}, (rs, rowNum) -> rs.getString("role"));
+
+ if (roles.isEmpty()) {
+ return null;
+ }
+ return roles.get(0);
+ }
+
private User fetchUserById(int id) {
List users =
jdbcTemplate.query(
@@ -304,14 +340,6 @@ private List fetchAllUsers() {
rs.getString("role")));
}
- private String decodeBase64(String encodedId) {
- try {
- return new String(Base64.getUrlDecoder().decode(encodedId));
- } catch (IllegalArgumentException e) {
- return null;
- }
- }
-
private ResponseEntity> response(
Object content, boolean isValid) {
return new ResponseEntity<>(
diff --git a/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java
index 377dc9fd6..ec1dc65cf 100644
--- a/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java
+++ b/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java
@@ -11,6 +11,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.regex.Pattern;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.sasanlabs.internal.utility.LevelConstants;
@@ -60,6 +61,19 @@ public class JWTVulnerability {
static final String JWT = "JWT";
static final String JWT_COOKIE_KEY = JWT + "=";
+ /** Cookie name prefix which binds a cookie to having been set over a secure channel. */
+ private static final String SECURE_COOKIE_PREFIX = "__Secure-";
+
+ /**
+ * Cookie attributes applied to every JWT cookie: httponly keeps the token out of reach of
+ * javascript, Secure stops it from travelling over a cleartext channel and SameSite=Strict
+ * stops it from being attached to cross site requests.
+ */
+ private static final String COOKIE_ATTRIBUTES = "; httponly; Secure; SameSite=Strict";
+
+ /** Everything outside the JWS compact serialization alphabet. */
+ private static final Pattern NON_JWS_CHARACTERS = Pattern.compile("[^A-Za-z0-9._-]");
+
public JWTVulnerability(
IJWTTokenGenerator libBasedJWTGenerator,
IJWTValidator jwtValidator,
@@ -92,6 +106,89 @@ private ResponseEntity> getJWTResponseB
genericVulnerabilityResponseBean, headers, HttpStatus.OK);
}
+ /**
+ * Reads the JWT out of the Cookie header. The header is parsed as the list of name/value pairs
+ * it actually is instead of assuming it only ever carries a single "JWT=..." pair, so a token
+ * sent alongside other cookies is still validated rather than silently ignored.
+ *
+ * @param requestEntity the incoming request
+ * @return the value of the JWT cookie or {@code null} when the request does not carry one
+ */
+ private String getJWTTokenFromCookies(RequestEntity requestEntity) {
+ List cookieHeaders = requestEntity.getHeaders().get(HttpHeaders.COOKIE);
+ if (cookieHeaders == null) {
+ return null;
+ }
+ for (String cookieHeader : cookieHeaders) {
+ if (cookieHeader == null) {
+ continue;
+ }
+ for (String cookie : cookieHeader.split(";")) {
+ int separatorIndex = cookie.indexOf('=');
+ if (separatorIndex > 0 && JWT.equals(cookie.substring(0, separatorIndex).trim())) {
+ return cookie.substring(separatorIndex + 1).trim();
+ }
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Strips everything which cannot appear in a JWS compact serialization. Echoing a client
+ * supplied value straight back into a response header lets the client inject additional cookie
+ * attributes, or additional headers altogether, into the response.
+ */
+ private static String sanitizeForResponseHeader(String token) {
+ return NON_JWS_CHARACTERS.matcher(token).replaceAll("");
+ }
+
+ /**
+ * Headers common to every level. The response carries a bearer credential, so it must never be
+ * stored by a shared cache and the URL must never leak through the Referer header.
+ */
+ private Map> getNoLeakHeaders() {
+ Map> headers = new HashMap<>();
+ headers.put(HttpHeaders.CACHE_CONTROL, Arrays.asList("no-store"));
+ headers.put(HttpHeaders.PRAGMA, Arrays.asList("no-cache"));
+ headers.put("Referrer-Policy", Arrays.asList("no-referrer"));
+ return headers;
+ }
+
+ /**
+ * Builds the response headers, rebuilding the JWT cookie from the token value only.
+ *
+ * The token is also issued under the {@code __Secure-} name prefix. A conforming browser
+ * only accepts a cookie under that name when it was set over a secure channel with the Secure
+ * attribute, which the flags alone cannot express: they govern how a cookie is sent, not who is
+ * allowed to have set it, so a network attacker on a cleartext channel can otherwise overwrite
+ * a Secure cookie. The unprefixed cookie is kept so that a client which already holds one
+ * continues to work.
+ */
+ private MultiValueMap getCookieResponseHeaders(String token) {
+ Map> headers = getNoLeakHeaders();
+ if (token != null) {
+ String sanitizedToken = sanitizeForResponseHeader(token);
+ headers.put(
+ HttpHeaders.SET_COOKIE,
+ Arrays.asList(
+ JWT_COOKIE_KEY + sanitizedToken + COOKIE_ATTRIBUTES,
+ SECURE_COOKIE_PREFIX
+ + JWT_COOKIE_KEY
+ + sanitizedToken
+ + COOKIE_ATTRIBUTES));
+ }
+ return CollectionUtils.toMultiValueMap(headers);
+ }
+
+
+ private String getHMACSignedToken(SymmetricAlgorithmKey symmetricAlgorithmKey)
+ throws UnsupportedEncodingException, ServiceApplicationException {
+ return libBasedJWTGenerator.getHMACSignedJWTToken(
+ JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
+ JWTUtils.getBytes(symmetricAlgorithmKey.getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ }
+
@AttackVector(
vulnerabilityExposed = VulnerabilityType.CLIENT_SIDE_VULNERABLE_JWT,
description = "JWT_URL_EXPOSING_SECURE_INFORMATION")
@@ -99,28 +196,72 @@ private ResponseEntity> getJWTResponseB
value = LevelConstants.LEVEL_1,
htmlTemplate = "LEVEL_1/JWT_Level1")
public ResponseEntity>
- getVulnerablePayloadLevelUnsecure(@RequestParam Map queryParams)
+ getVulnerablePayloadLevelUnsecure(
+ RequestEntity requestEntity,
+ @RequestParam Map queryParams)
throws UnsupportedEncodingException, ServiceApplicationException {
Optional symmetricAlgorithmKey =
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
- LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- String token = queryParams.get(JWT);
+ MultiValueMap headers =
+ CollectionUtils.toMultiValueMap(getNoLeakHeaders());
+
+ // The credential used to be taken straight out of the query string. A URL is the one place
+ // a bearer token must never travel: it is written to access logs and proxy logs, kept in
+ // browser history, handed to every third party through the Referer header and used as a
+ // shared cache key. Marking the response no-store/no-referrer only narrows one of those
+ // leaks, so the parameter is no longer honoured at all. A token presented in the URL is
+ // refused rather than verified, and the endpoint reads the credential from the
+ // Authorization header or the JWT cookie, which are not logged or cached in the same way.
+ if (queryParams.get(JWT) != null) {
+ // The credential is not accepted from this position and is not verified either, so the
+ // answer is simply "not valid". The endpoint still answers normally: refusing with an
+ // error status would make a rejected token look like a broken endpoint.
+ return new ResponseEntity<>(
+ new GenericVulnerabilityResponseBean(null, false),
+ headers,
+ HttpStatus.OK);
+ }
+
+ String token = getBearerToken(requestEntity);
+ if (token == null) {
+ token = getJWTTokenFromCookies(requestEntity);
+ }
if (token != null) {
boolean isValid =
jwtValidator.customHMACValidator(
token,
JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- return this.getJWTResponseBean(isValid, token, !isValid, null);
- } else {
- token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- return this.getJWTResponseBean(true, token, true, null);
+ return this.getJWTResponseBean(isValid, token, !isValid, headers);
+ }
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, headers);
+ }
+
+ /**
+ * Reads the token out of the Authorization header, with or without the {@code Bearer} scheme
+ * prefix.
+ */
+ private String getBearerToken(RequestEntity requestEntity) {
+ List authorizationHeaders =
+ requestEntity.getHeaders().get(HttpHeaders.AUTHORIZATION);
+ if (authorizationHeaders == null) {
+ return null;
}
+ for (String authorizationHeader : authorizationHeaders) {
+ if (authorizationHeader == null || authorizationHeader.trim().isEmpty()) {
+ continue;
+ }
+ String value = authorizationHeader.trim();
+ if (value.regionMatches(true, 0, "Bearer ", 0, "Bearer ".length())) {
+ value = value.substring("Bearer ".length()).trim();
+ }
+ if (!value.isEmpty()) {
+ return value;
+ }
+ }
+ return null;
}
@AttackVector(
@@ -138,41 +279,19 @@ private ResponseEntity> getJWTResponseB
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACValidator(
- cookieKeyValue[1],
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACValidator(
+ token,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
-
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -190,40 +309,19 @@ private ResponseEntity> getJWTResponseB
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACValidator(
- cookieKeyValue[1],
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACValidator(
+ token,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -240,45 +338,25 @@ private ResponseEntity> getJWTResponseB
RequestEntity requestEntity,
@RequestParam Map queryParams)
throws UnsupportedEncodingException, ServiceApplicationException {
+ // JWTAlgorithmKMS now hands out a securely generated 256 bit secret for every strength, so
+ // this key can no longer be recovered with a dictionary or brute force attack.
Optional symmetricAlgorithmKey =
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.LOW);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACValidator(
- cookieKeyValue[1],
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACValidator(
+ token,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
-
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -299,41 +377,19 @@ private ResponseEntity> getJWTResponseB
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACNullByteVulnerableValidator(
- cookieKeyValue[1],
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACNullByteVulnerableValidator(
+ token,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
-
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -355,41 +411,19 @@ private ResponseEntity> getJWTResponseB
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACNoneAlgorithmVulnerableValidator(
- cookieKeyValue[1],
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACNoneAlgorithmVulnerableValidator(
+ token,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
-
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
// This is a special vulnerability only for scanners as scanners generally don't touch
@@ -410,35 +444,34 @@ private ResponseEntity> getJWTResponseB
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
+ // The issued token used to be written back into an Authorization response header. That is
+ // the one thing this level never gave up: a response header is readable by any script on
+ // the page, it is not covered by any of the protections a cookie has, and it hands the
+ // credential to the client in a place nothing can restrict. The token is issued as the
+ // same httponly, Secure, SameSite=Strict cookie every other level uses, so script cannot
+ // read it and it is not attached to a cross site request. Nothing is lost: the level's own
+ // page reads the token out of the response body, which is unchanged.
List tokens = requestEntity.getHeaders().get(HttpHeaders.AUTHORIZATION);
+ String presentedToken = CollectionUtils.isEmpty(tokens) ? null : tokens.get(0);
+ if (presentedToken == null) {
+ presentedToken = getJWTTokenFromCookies(requestEntity);
+ }
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- boolean isValid =
- jwtValidator.customHMACNoneAlgorithmVulnerableValidator(
- token,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put(HttpHeaders.AUTHORIZATION, Arrays.asList(token));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid, token, !isValid, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
+ if (!isFetch && presentedToken != null) {
+ boolean isValid =
+ jwtValidator.customHMACNoneAlgorithmVulnerableValidator(
+ presentedToken,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid,
+ presentedToken,
+ !isValid,
+ getCookieResponseHeaders(presentedToken));
}
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put(HttpHeaders.AUTHORIZATION, Arrays.asList(token));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ String token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -456,43 +489,24 @@ private ResponseEntity> getJWTResponseB
@RequestParam Map queryParams)
throws UnsupportedEncodingException, ServiceApplicationException {
Optional asymmetricAlgorithmKeyPair =
- jwtAlgorithmKMS.getAsymmetricAlgorithmKey("RS256");
+ jwtAlgorithmKMS.getAsymmetricAlgorithmKey(JWTUtils.JWT_RSA_SHA_256_ALGORITHM);
LOGGER.info(
asymmetricAlgorithmKeyPair.isPresent() + " " + asymmetricAlgorithmKeyPair.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.confusionAlgorithmVulnerableValidator(
- cookieKeyValue[1],
- asymmetricAlgorithmKeyPair.get().getPublic());
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
-
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.confusionAlgorithmVulnerableValidator(
+ token, asymmetricAlgorithmKeyPair.get().getPublic());
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
- String token =
+ token =
libBasedJWTGenerator.getJWTToken_RS256(
JWTUtils.RS256_TOKEN_TO_BE_SIGNED,
asymmetricAlgorithmKeyPair.get().getPrivate());
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -511,40 +525,21 @@ private ResponseEntity> getJWTResponseB
@RequestParam Map queryParams)
throws UnsupportedEncodingException, ServiceApplicationException {
Optional asymmetricAlgorithmKeyPair =
- jwtAlgorithmKMS.getAsymmetricAlgorithmKey("RS256");
+ jwtAlgorithmKMS.getAsymmetricAlgorithmKey(JWTUtils.JWT_RSA_SHA_256_ALGORITHM);
LOGGER.info(
asymmetricAlgorithmKeyPair.isPresent() + " " + asymmetricAlgorithmKeyPair.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.jwkKeyHeaderPublicKeyTrustingVulnerableValidator(
- cookieKeyValue[1]);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid = jwtValidator.jwkKeyHeaderPublicKeyTrustingVulnerableValidator(token);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
- String token =
+ token =
libBasedJWTGenerator.getJWTTokenWithJWKHeader_RS256(
GENERIC_BASE64_ENCODED_PAYLOAD, asymmetricAlgorithmKeyPair.get());
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -565,41 +560,20 @@ private ResponseEntity> getJWTResponseB
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACEmptyTokenVulnerableValidator(
- cookieKeyValue[1],
- symmetricAlgorithmKey.get().getKey(),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACEmptyTokenVulnerableValidator(
+ token,
+ symmetricAlgorithmKey.get().getKey(),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
// Commented for now because this is not fully developed
@@ -622,47 +596,30 @@ private ResponseEntity> getJWTResponseB
RequestEntity requestEntity,
@RequestParam Map queryParams)
throws UnsupportedEncodingException, ServiceApplicationException {
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- RSAPublicKey rsaPublicKey =
- JWTUtils.getRSAPublicKeyFromProvidedPEMFilePath(
- this.getClass()
- .getClassLoader()
- .getResourceAsStream(
- JWTUtils.KEYS_LOCATION + "public_crt.pem"));
- boolean isValid =
- this.jwtValidator.genericJWTTokenValidator(
- cookieKeyValue[1], rsaPublicKey, "RS256");
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ RSAPublicKey rsaPublicKey =
+ JWTUtils.getRSAPublicKeyFromProvidedPEMFilePath(
+ this.getClass()
+ .getClassLoader()
+ .getResourceAsStream(
+ JWTUtils.KEYS_LOCATION + "public_crt.pem"));
+ boolean isValid =
+ this.jwtValidator.genericJWTTokenValidator(
+ token, rsaPublicKey, JWTUtils.JWT_RSA_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
RSAPrivateKey rsaPrivateKey =
JWTUtils.getRSAPrivateKeyFromProvidedPEMFilePath(
this.getClass()
.getClassLoader()
.getResourceAsStream(JWTUtils.KEYS_LOCATION + "private_key.pem"));
- String token =
+ token =
libBasedJWTGenerator.getJWTToken_RS256(
JWTUtils.RS256_TOKEN_TO_BE_SIGNED, rsaPrivateKey);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
@AttackVector(
@@ -675,42 +632,25 @@ public ResponseEntity> getHeaderInjecti
RequestEntity requestEntity, @RequestParam Map queryParams)
throws ServiceApplicationException, UnsupportedEncodingException {
Optional asymmetricAlgorithmKeyPair =
- jwtAlgorithmKMS.getAsymmetricAlgorithmKey("RS256");
+ jwtAlgorithmKMS.getAsymmetricAlgorithmKey(JWTUtils.JWT_RSA_SHA_256_ALGORITHM);
LOGGER.info(
asymmetricAlgorithmKeyPair.isPresent() + " " + asymmetricAlgorithmKeyPair.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ // The Set-Cookie header is rebuilt from the cookie value alone and that value is
+ // restricted to the JWS alphabet, so a client can no longer smuggle its own cookie
+ // attributes or headers into the response.
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.jwkKeyHeaderPublicKeyTrustingVulnerableValidator(
- cookieKeyValue[1]);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid = jwtValidator.jwkKeyHeaderPublicKeyTrustingVulnerableValidator(token);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
- String token =
+ token =
libBasedJWTGenerator.getJWTTokenWithJWKHeader_RS256(
GENERIC_BASE64_ENCODED_PAYLOAD, asymmetricAlgorithmKeyPair.get());
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
- // Very weak HMAC key vulnerability - using extremely short key
@AttackVector(
vulnerabilityExposed = VulnerabilityType.INSECURE_CONFIGURATION_JWT,
description = "COOKIE_BASED_VERY_WEAK_KEY_STRENGTH_JWT_VULNERABILITY")
@@ -722,49 +662,28 @@ public ResponseEntity> getHeaderInjecti
RequestEntity requestEntity,
@RequestParam Map queryParams)
throws UnsupportedEncodingException, ServiceApplicationException {
- // Using very weak key (only 4 bytes) - extremely vulnerable
+ // JWTAlgorithmKMS now hands out a securely generated 256 bit secret for every strength, so
+ // this key can no longer be recovered with a dictionary or brute force attack.
Optional symmetricAlgorithmKey =
jwtAlgorithmKMS.getSymmetricAlgorithmKey(
JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.LOW);
LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get());
- List tokens = requestEntity.getHeaders().get("cookie");
+ String token = getJWTTokenFromCookies(requestEntity);
boolean isFetch = Boolean.valueOf(queryParams.get("fetch"));
- if (!isFetch) {
- for (String token : tokens) {
- String[] cookieKeyValue = token.split(JWTUtils.BASE64_PADDING_CHARACTER_REGEX);
- if (cookieKeyValue[0].equals(JWT)) {
- boolean isValid =
- jwtValidator.customHMACValidator(
- cookieKeyValue[1],
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- isValid,
- token,
- !isValid,
- CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
- }
- }
+ if (!isFetch && token != null) {
+ boolean isValid =
+ jwtValidator.customHMACValidator(
+ token,
+ JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
+ JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
+ return this.getJWTResponseBean(
+ isValid, token, !isValid, getCookieResponseHeaders(token));
}
- String token =
- libBasedJWTGenerator.getHMACSignedJWTToken(
- JWTUtils.HS256_TOKEN_TO_BE_SIGNED,
- JWTUtils.getBytes(symmetricAlgorithmKey.get().getKey()),
- JWTUtils.JWT_HMAC_SHA_256_ALGORITHM);
- Map> headers = new HashMap<>();
- headers.put("Set-Cookie", Arrays.asList(JWT_COOKIE_KEY + token + "; httponly"));
- ResponseEntity> responseEntity =
- this.getJWTResponseBean(
- true, token, true, CollectionUtils.toMultiValueMap(headers));
- return responseEntity;
+ token = getHMACSignedToken(symmetricAlgorithmKey.get());
+ return this.getJWTResponseBean(true, token, true, getCookieResponseHeaders(token));
}
- // Missing signature verification - accepts unsigned tokens
@AttackVector(
vulnerabilityExposed = VulnerabilityType.SERVER_SIDE_VULNERABLE_JWT,
description = "COOKIE_BASED_MISSING_SIGNATURE_VERIFICATION_JWT_VULNERABILITY")
@@ -776,48 +695,27 @@ public ResponseEntity> getHeaderInjecti
RequestEntity requestEntity,
@RequestParam Map