Harden graded VulnerableApp vulnerability levels

src/main/java/org/sasanlabs/controller/exception/ControllerExceptionHandler.java

@@ -9,6 +9,7 @@
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.context.request.WebRequest;
+import org.springframework.web.multipart.MaxUploadSizeExceededException;
 import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
 
 /**
@@ -37,6 +38,20 @@ public ResponseEntity<String> handleControllerExceptions(
                 HttpStatus.INTERNAL_SERVER_ERROR);
     }
 
+    /**
+     * An upload larger than the configured ceiling is a rejected request, not a server fault.
+     * Letting it fall through to the catch-all below answered 500, which both reports a defect that
+     * does not exist and hides the size limit that is doing its job.
+     */
+    @ExceptionHandler(MaxUploadSizeExceededException.class)
+    public ResponseEntity<String> handleUploadTooLarge(
+            MaxUploadSizeExceededException ex, WebRequest request) {
+        LOGGER.info("Rejected an upload exceeding the configured maximum size");
+        return new ResponseEntity<String>(
+                "Input is invalid: file exceeds the maximum permitted size",
+                HttpStatus.PAYLOAD_TOO_LARGE);
+    }
+
     @ExceptionHandler(Exception.class)
     public ResponseEntity<String> handleExceptions(Exception ex, WebRequest request) {
         LOGGER.error("General Exception Occurred :- ", ex);

src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthLoginService.java

@@ -36,38 +36,46 @@ public AuthLoginService(
         this.passwordEncoder = passwordEncoder;
     }
 
-    /** Level 1: SQL Injection. Demonstrates a login query vulnerable to string concatenation. */
+    /**
+     * Level 1: the credentials are bound as query parameters, so a value such as {@code ' OR
+     * '1'='1} is compared as a literal string instead of being parsed as SQL. The database error is
+     * also no longer echoed back, since it would otherwise hand an attacker error-based SQL
+     * injection.
+     */
     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
-                        + "'";
+        String sql = "SELECT * FROM auth_users WHERE level=1 AND username=? AND password=?";
         try {
-            // Level 1 still uses JdbcTemplate to allow SQL Injection bypass
             List<AuthUser> users =
-                    jdbcTemplate.query(sql, new BeanPropertyRowMapper<>(AuthUser.class));
+                    jdbcTemplate.query(
+                            sql, new BeanPropertyRowMapper<>(AuthUser.class), username, password);
             if (!users.isEmpty()) {
                 return AuthResult.success(users.get(0));
             }
         } catch (Exception e) {
-            // In a real exploit, this might be an error-based SQLi
-            return AuthResult.failure("Database error: " + e.getMessage());
+            LOGGER.error("Login query failed for level 1", e);
+            return AuthResult.failure("Invalid credentials");
         }
         return AuthResult.failure("Invalid credentials");
     }
 
-    /** Level 2: Sensitive Data Logging. Logs the provided password to the logs. */
+    /**
+     * Level 2: the attempt is still logged for audit purposes, but the submitted password is not.
+     * Anyone with read access to the logs would otherwise hold every password typed at this form,
+     * including the near-misses that reveal a real password by a character or two.
+     */
     public AuthResult authenticateLevel2Logging(String username, String password) {
         Optional<AuthUser> userOpt = authUserRepository.findByUsernameAndLevel(username, 2);
 
-        LOGGER.info("Login attempt for user: {} | provided password: {}", username, password);
+        LOGGER.info("Login attempt for user: {}", username);
 
+        /*
+         * The stored value is a BCrypt hash, not the password, so the comparison goes through
+         * the encoder. A direct string equality check against the column only works while the
+         * credential is held in the clear -- which is the weakness this level is named for.
+         */
         if (userOpt.isPresent()
                 && password != null
-                && password.equals(userOpt.get().getPassword())) {
+                && passwordEncoder.matches(password, userOpt.get().getPassword())) {
             return AuthResult.success(userOpt.get());
         }
         return AuthResult.failure("Invalid credentials");
@@ -78,9 +86,13 @@ public AuthResult authenticate(String username, String password, int level) {
         return authenticateInternal(username, password, level, false);
     }
 
-    /** Authentication method that intentionally exposes username enumeration behavior. */
+    /**
+     * Previously distinguished "User not found" from "Invalid password", which let an attacker
+     * confirm valid usernames one request at a time and build a target list before ever guessing a
+     * password. Both outcomes now report the same generic failure.
+     */
     public AuthResult authenticateWithEnumeration(String username, String password, int level) {
-        return authenticateInternal(username, password, level, true);
+        return authenticateInternal(username, password, level, false);
     }
 
     private AuthResult authenticateInternal(

src/main/java/org/sasanlabs/service/vulnerability/authentication/AuthenticationVulnerability.java

@@ -141,9 +141,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<Object>> level3Plaintext(
             return response(result.getErrorMessage(), false);
         }
         // Exposure of plaintext password
-        Map<String, Object> profile = buildProfile(result.getUser());
-        profile.put("passwordInDB", result.getUser().getPassword());
-        return response(profile, true);
+        return response(buildProfile(result.getUser()), true);
     }
 
     // ------------------------------------------------------------------ Level 4 — MD5
@@ -397,10 +395,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<Object>> level10LowIterat
         if (!result.isAuthenticated()) {
             return response(result.getErrorMessage(), false);
         }
-        Map<String, Object> profile = buildProfile(result.getUser());
-        profile.put("passwordHash", result.getUser().getPassword());
-        profile.put("algorithm", "BCrypt (Cost: 4)");
-        return response(profile, true);
+        return response(buildProfile(result.getUser()), true);
     }
 
     // ------------------------------------------------------------------ Helpers

src/main/java/org/sasanlabs/service/vulnerability/cachePoisoning/CachePoisoningVulnerability.java

@@ -82,7 +82,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> getVulnerablePay
             HttpServletRequest request) {
         String responseContent = buildLevel1Response(banner);
         return buildCachedResponse(
-                buildRouteOnlyCacheKey(request),
+                buildRouteAndBannerCacheKey(request, banner),
                 responseContent,
                 resolvePublicCacheControl(browserCache),
                 true);
@@ -106,7 +106,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> getVulnerablePay
             HttpServletRequest request) {
         String responseContent = buildLevel2Response(banner);
         return buildCachedResponse(
-                buildRouteOnlyCacheKey(request),
+                buildRouteAndBannerCacheKey(request, banner),
                 responseContent,
                 resolvePublicCacheControl(browserCache),
                 true);
@@ -148,11 +148,13 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> getVulnerablePay
                     boolean browserCache,
             HttpServletRequest request) {
         String responseContent = buildLevel4Response(request);
+        // The dashboard is derived from the caller's own cookie, so it must never be written to a
+        // shared cache -- whoever asked next would be served the previous user's profile.
         return buildCachedResponse(
-                buildRouteOnlyCacheKey(request),
+                buildPrivateResponseKey(request),
                 responseContent,
-                resolvePublicCacheControl(browserCache),
-                true);
+                CACHE_CONTROL_PRIVATE_NO_STORE,
+                false);
     }
 
     @AttackVector(
@@ -175,32 +177,37 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> getSecurePayload
     }
 
     private String buildLevel1Response(String banner) {
-        String unsafeBanner = StringUtils.defaultIfBlank(banner, DEFAULT_BANNER);
+        String safeBanner =
+                StringEscapeUtils.escapeHtml4(StringUtils.defaultIfBlank(banner, DEFAULT_BANNER));
         return "<section class=\"cache-poisoning-response\">"
                 + "<h3>Shared Cache Response</h3>"
                 + "<p><strong>Current Banner:</strong> "
-                + unsafeBanner
+                + safeBanner
                 + "</p>"
-                + "<p>The application reflects the <code>banner</code> parameter, but the cache only uses the route as the key.</p>"
-                + "<p>Try poisoning the banner and see if it persists for other requests.</p>"
+                + "<p>The <code>banner</code> parameter is escaped before it is reflected and forms part of the cache key.</p>"
                 + "</section>";
     }
 
     private String buildLevel2Response(String banner) {
-        String filteredBanner = applyNaiveBannerFilter(banner);
+        // Escaping replaces the tag stripping filter rather than supplementing it: a blocklist that
+        // removes <script> still lets through <img src=x onerror=...> and every other event
+        // handler.
+        String safeBanner =
+                StringEscapeUtils.escapeHtml4(StringUtils.defaultIfBlank(banner, DEFAULT_BANNER));
         return "<section class=\"cache-poisoning-response\">"
                 + "<h3>Filtered Cache Response</h3>"
                 + "<p><strong>Current Banner:</strong> "
-                + filteredBanner
+                + safeBanner
                 + "</p>"
-                + "<p>Obvious <code>&lt;script&gt;</code> tags are stripped, but the cache key remains route-only.</p>"
-                + "<p>Can you still poison the cache with other HTML or misleading information?</p>"
+                + "<p>The banner is HTML escaped rather than filtered, and forms part of the cache key.</p>"
                 + "</section>";
     }
 
     private String buildLevel3Response(String banner, HttpServletRequest request) {
         String safeBanner = StringEscapeUtils.escapeHtml4(normalizeBanner(banner));
-        String assetUrl = buildAssetUrl(resolveUntrustedForwardedHost(request));
+        // X-Forwarded-Host is attacker supplied and is not part of the cache key, so a single
+        // poisoned request would pin every later visitor's asset URL to the attacker's host.
+        String assetUrl = buildAssetUrl(TRUSTED_ASSET_HOST);
         return "<section class=\"cache-poisoning-response\">"
                 + "<h3>Dynamic Asset Loading</h3>"
                 + "<p><strong>Banner Key:</strong> "
@@ -218,8 +225,7 @@ private String buildLevel3Response(String banner, HttpServletRequest request) {
                 + "\" class=\"asset-preview-iframe\"></iframe>"
                 + "  <p class=\"asset-preview-note\">The browser is attempting to load the resource from the host above. Use the <b>Network Tab</b> to verify the origin.</p>"
                 + "</div>"
-                + "<p>The <code>banner</code> is now part of the cache key, but the application trusts the <code>X-Forwarded-Host</code> header for asset URLs.</p>"
-                + "<p>If the cache ignores this header, the asset location can be poisoned.</p>"
+                + "<p>The <code>banner</code> is part of the cache key and asset URLs are built from a fixed trusted host, not from a request header.</p>"
                 + "</section>";
     }
 
@@ -243,8 +249,7 @@ private String buildLevel4ResponseForUser(String user) {
                 + "<p><strong>Last login IP:</strong> "
                 + safeIp
                 + "</p>"
-                + "<p>This response is personalized based on your session cookie but is marked as <code>public</code>.</p>"
-                + "<p>Check if your personalized dashboard appears for other users due to shared cache reuse.</p>"
+                + "<p>This response is personalized, so it is marked <code>private, no-store</code> and never written to a shared cache.</p>"
                 + "</section>";
     }
 

src/main/java/org/sasanlabs/service/vulnerability/clickjacking/ClickjackingVulnerability.java

@@ -33,12 +33,29 @@
         value = "ClickjackingVulnerability")
 public class ClickjackingVulnerability {
 
-    private static final String VULNERABLE_RESPONSE =
-            "Page loaded without framing protection. This page can be embedded in an iframe.";
-
     private static final String PROTECTED_RESPONSE =
             "Page loaded with framing protection header set.";
 
+    private static final String X_FRAME_OPTIONS = "X-Frame-Options";
+
+    private static final String CONTENT_SECURITY_POLICY = "Content-Security-Policy";
+
+    /**
+     * Sends both framing defenses at once, as the OWASP Clickjacking Defense Cheat Sheet
+     * recommends: {@code frame-ancestors 'none'} is the modern control, and {@code X-Frame-Options:
+     * DENY} still covers browsers that do not honour it. {@code SAMEORIGIN} is not enough on its
+     * own -- it leaves the same-origin overlay attack of levels 6 and 7 wide open -- and {@code
+     * ALLOWALL} is not a real directive, so it protects nothing at all.
+     */
+    private ResponseEntity<GenericVulnerabilityResponseBean<String>> framingProtectedResponse() {
+        HttpHeaders headers = new HttpHeaders();
+        headers.add(X_FRAME_OPTIONS, "DENY");
+        headers.add(CONTENT_SECURITY_POLICY, "frame-ancestors 'none'");
+        return ResponseEntity.ok()
+                .headers(headers)
+                .body(new GenericVulnerabilityResponseBean<>(PROTECTED_RESPONSE, true));
+    }
+
     /**
      * 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 +79,7 @@ public class ClickjackingVulnerability {
             value = LevelConstants.LEVEL_1,
             htmlTemplate = "LEVEL_1/ClickjackingVulnerability")
     public ResponseEntity<GenericVulnerabilityResponseBean<String>> noFramingProtection() {
-        return ResponseEntity.ok(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+        return this.framingProtectedResponse();
     }
 
     /**
@@ -88,11 +105,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> noFramingProtect
             value = LevelConstants.LEVEL_2,
             htmlTemplate = "LEVEL_1/ClickjackingVulnerability")
     public ResponseEntity<GenericVulnerabilityResponseBean<String>> xFrameOptionsAllowAll() {
-        HttpHeaders headers = new HttpHeaders();
-        headers.add("X-Frame-Options", "ALLOWALL");
-        return ResponseEntity.ok()
-                .headers(headers)
-                .body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+        return this.framingProtectedResponse();
     }
 
     /**
@@ -118,11 +131,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> xFrameOptionsAll
             value = LevelConstants.LEVEL_3,
             htmlTemplate = "LEVEL_1/ClickjackingVulnerability")
     public ResponseEntity<GenericVulnerabilityResponseBean<String>> xFrameOptionsSameOrigin() {
-        HttpHeaders headers = new HttpHeaders();
-        headers.add("X-Frame-Options", "SAMEORIGIN");
-        return ResponseEntity.ok()
-                .headers(headers)
-                .body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+        return this.framingProtectedResponse();
     }
 
     /**
@@ -181,7 +190,7 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> cspFrameAncestor
             value = LevelConstants.LEVEL_6,
             htmlTemplate = "LEVEL_4/ClickjackingVulnerability")
     public ResponseEntity<GenericVulnerabilityResponseBean<String>> overlayAttackNoProtection() {
-        return ResponseEntity.ok(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+        return this.framingProtectedResponse();
     }
 
     /**
@@ -208,10 +217,6 @@ public ResponseEntity<GenericVulnerabilityResponseBean<String>> overlayAttackNoP
             value = LevelConstants.LEVEL_7,
             htmlTemplate = "LEVEL_4/ClickjackingVulnerability")
     public ResponseEntity<GenericVulnerabilityResponseBean<String>> overlayAttackSameOrigin() {
-        HttpHeaders headers = new HttpHeaders();
-        headers.add("X-Frame-Options", "SAMEORIGIN");
-        return ResponseEntity.ok()
-                .headers(headers)
-                .body(new GenericVulnerabilityResponseBean<>(VULNERABLE_RESPONSE, true));
+        return this.framingProtectedResponse();
     }
 }