Fix VulnerableApp vulnerabilities (remediation)

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}.
+     *
+     * <p>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.
+     *
+     * <p>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);
+            }
+        };
+    }
 }

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.
 }

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.
-     *
-     * <p>Algorithm based on the LAN Manager specification.
-     *
-     * @see <a href="https://en.wikipedia.org/wiki/LAN_Manager">Wikipedia: LAN Manager</a>
-     */
-    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));
-    }
 }