Harden SecurePasswords assignment input/output handling and remove per-request Zxcvbn initialization

src/main/java/org/owasp/webgoat/lessons/securepasswords/SecurePasswordsAssignment.java

@@ -18,28 +18,37 @@
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.util.HtmlUtils;
 
 @RestController
 public class SecurePasswordsAssignment implements AssignmentEndpoint {
 
+  private static final Zxcvbn ZXCVBN = new Zxcvbn();
+
   @PostMapping("SecurePasswords/assignment")
   @ResponseBody
   public AttackResult completed(@RequestParam String password) {
-    Zxcvbn zxcvbn = new Zxcvbn();
+    if (password == null || password.trim().isEmpty()) {
+      return failed(this)
+          .feedback("securepassword-failed")
+          .output("<b>Error:</b> Password must not be empty.")
+          .build();
+    }
+
     StringBuilder output = new StringBuilder();
     DecimalFormat df = new DecimalFormat("0", DecimalFormatSymbols.getInstance(Locale.ENGLISH));
     df.setMaximumFractionDigits(340);
-    Strength strength = zxcvbn.measure(password);
+    Strength strength = ZXCVBN.measure(password);
 
     output.append("<b>Your Password: *******</b></br>");
-    output.append("<b>Length: </b>" + password.length() + "</br>");
+    output.append("<b>Length: </b>" + HtmlUtils.htmlEscape(String.valueOf(password.length())) + "</br>");
     output.append(
         "<b>Estimated guesses needed to crack your password: </b>"
-            + df.format(strength.getGuesses())
+            + HtmlUtils.htmlEscape(df.format(strength.getGuesses()))
             + "</br>");
     output.append(
         "<div style=\"float: left;padding-right: 10px;\"><b>Score: </b>"
-            + strength.getScore()
+            + HtmlUtils.htmlEscape(String.valueOf(strength.getScore()))
             + "/4 </div>");
     if (strength.getScore() <= 1) {
       output.append(
@@ -56,24 +65,26 @@ public AttackResult completed(@RequestParam String password) {
     }
     output.append(
         "<b>Estimated cracking time: </b>"
-            + calculateTime(
-                (long) strength.getCrackTimeSeconds().getOnlineNoThrottling10perSecond())
+            + HtmlUtils.htmlEscape(
+                calculateTime(
+                    (long) strength.getCrackTimeSeconds().getOnlineNoThrottling10perSecond()))
             + "</br>");
-      output.append(
-              "<i>Note:</i> This estimate assumes brute-force attack and does not account for "
-                      + "dictionary or rule-based attacks, which can significantly reduce real-world cracking time "
-                      + "for common phrases.</br>");
+    output.append(
+        "<i>Note:</i> This estimate assumes brute-force attack and does not account for "
+            + "dictionary or rule-based attacks, which can significantly reduce real-world cracking time "
+            + "for common phrases.</br>");
     if (strength.getFeedback().getWarning().length() != 0)
-      output.append("<b>Warning: </b>" + strength.getFeedback().getWarning() + "</br>");
+      output.append(
+          "<b>Warning: </b>" + HtmlUtils.htmlEscape(strength.getFeedback().getWarning()) + "</br>");
     // possible feedback: https://github.com/dropbox/zxcvbn/blob/master/src/feedback.coffee
     // maybe ask user to try also weak passwords to see and understand feedback?
     if (strength.getFeedback().getSuggestions().size() != 0) {
       output.append("<b>Suggestions:</b></br><ul>");
       for (String sug : strength.getFeedback().getSuggestions())
-        output.append("<li>" + sug + "</li>");
+        output.append("<li>" + HtmlUtils.htmlEscape(sug) + "</li>");
       output.append("</ul></br>");
     }
-    output.append("<b>Score: </b>" + strength.getScore() + "/4 </br>");
+    output.append("<b>Score: </b>" + HtmlUtils.htmlEscape(String.valueOf(strength.getScore())) + "/4 </br>");
 
     if (strength.getScore() >= 4)
       return success(this).feedback("securepassword-success").output(output.toString()).build();
@@ -91,7 +102,7 @@ public static String calculateTime(long seconds) {
     long days = (seconds % yr) / (d);
     long hours = (seconds % d) / (hr);
     long minutes = (seconds % hr) / (min);
-    long sec = (seconds % min * s);
+    long sec = seconds % min;
 
     return (years
         + " years "

src/test/java/org/owasp/webgoat/lessons/securepasswords/SecurePasswordsAssignmentTest.java

@@ -0,0 +1,48 @@
+/*
+ * SPDX-FileCopyrightText: Copyright © 2026 WebGoat authors
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+package org.owasp.webgoat.lessons.securepasswords;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.Test;
+import org.owasp.webgoat.container.assignments.AttackResult;
+
+class SecurePasswordsAssignmentTest {
+
+  private final SecurePasswordsAssignment assignment = new SecurePasswordsAssignment();
+
+  @Test
+  void shouldFailWhenPasswordIsNull() {
+    AttackResult result = assignment.completed(null);
+
+    assertThat(result.assignmentSolved()).isFalse();
+    assertThat(result.getFeedback()).isEqualTo("securepassword-failed");
+    assertThat(result.getOutput()).contains("Password must not be empty.");
+  }
+
+  @Test
+  void shouldFailWhenPasswordIsBlank() {
+    AttackResult result = assignment.completed("   ");
+
+    assertThat(result.assignmentSolved()).isFalse();
+    assertThat(result.getFeedback()).isEqualTo("securepassword-failed");
+    assertThat(result.getOutput()).contains("Password must not be empty.");
+  }
+
+  @Test
+  void shouldMaskPasswordInResponseOutput() {
+    String password = "<script>alert(1)</script>VeryStrongPassword!123";
+    AttackResult result = assignment.completed(password);
+
+    assertThat(result.getOutput()).contains("<b>Your Password: *******</b>");
+    assertThat(result.getOutput()).doesNotContain(password);
+  }
+
+  @Test
+  void calculateTimeShouldReturnExpectedSecondsRemainder() {
+    assertThat(SecurePasswordsAssignment.calculateTime(61))
+        .isEqualTo("0 years 0 days 0 hours 1 minutes 1 seconds");
+  }
+}