Fix href attribute-injection XSS in Cross Site Scripting Challenge 5

src/main/java/utils/XssFilter.java

@@ -77,8 +77,20 @@ public static String badUrlValidate(String input) {
                     .replaceAll("#", "#")
                     .replaceAll("<", "&#x3c;")
                     .replaceAll(">", "&#x3e;")
-                    .replaceFirst("\"", "&quot;"));
-        input = theUrl.toString();
+                    // Every double quote must be neutralised, not just the first one - this value
+                    // is dropped straight into a double-quoted href="" attribute, so a second,
+                    // un-encoded quote lets an attacker close the attribute early and append a
+                    // brand new attribute (e.g. an onmouseover/onclick handler) to the <a> tag.
+                    .replaceAll("\"", "&quot;"));
+        // Only genuine http/https links should ever come out of a "URL validator" - reject
+        // anything else in case URL parsing/normalisation ever produces another scheme.
+        String protocol = theUrl.getProtocol();
+        if ("http".equalsIgnoreCase(protocol) || "https".equalsIgnoreCase(protocol)) {
+          input = theUrl.toString();
+        } else {
+          log.debug("Rejected non-HTTP(S) protocol after parsing: " + protocol);
+          input = howToMakeAUrlUrl;
+        }
       } catch (MalformedURLException e) {
         log.debug("Could not Cast URL from input: " + e.toString());
         input = howToMakeAUrlUrl;

src/test/java/utils/XssChallengeFiveExploitTest.java

@@ -0,0 +1,65 @@
+package utils;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Test;
+
+// Reproduces the XssChallengeFive.doPost() output-construction chain: userPost is
+// "<a href=\"" + badUrlValidate(searchTerm) + "\">Your HTTP Link!</a>", exercised here
+// without needing the full servlet container.
+//
+// The pre-fix badUrlValidate() only HTML-encoded the FIRST double quote it found
+// (replaceFirst instead of replaceAll) before splicing the result into a double-quoted
+// href="" attribute. A payload with a harmless decoy quote up front left a later, real
+// quote un-encoded, letting an attacker close the href attribute early and append a new
+// onmouseover/onerror/onclick attribute - HTML parsers accept a new attribute right after
+// a closing quote even with no whitespace in between. Confirmed (outside this test, via a
+// standalone harness) that this exact payload shape scored FindXSS.search() == true against
+// the original code; these tests assert the fixed code closes it off while legitimate
+// http(s) links keep working.
+class XssChallengeFiveExploitTest {
+
+  private static String buildUserPost(String searchTerm) {
+    String validated = XssFilter.badUrlValidate(searchTerm);
+    return "<a href=\"" + validated + "\">Your HTTP Link!</a>";
+  }
+
+  @Test
+  void decoyQuoteOnmouseoverBreakout_noLongerDetectedAsXss() {
+    String payload = "http:\"foo\"onmouseover=\"alert(1)";
+    String userPost = buildUserPost(payload);
+    assertFalse(FindXSS.search(userPost), "onmouseover attribute-breakout should be neutralised");
+  }
+
+  @Test
+  void decoyQuoteOnerrorBreakout_noLongerDetectedAsXss() {
+    String payload = "http:\"foo\"onerror=\"alert(1)";
+    String userPost = buildUserPost(payload);
+    assertFalse(FindXSS.search(userPost), "onerror attribute-breakout should be neutralised");
+  }
+
+  @Test
+  void decoyQuoteOnclickBreakout_noLongerDetectedAsXss() {
+    String payload = "https:\"foo\"onclick=\"alert(1)";
+    String userPost = buildUserPost(payload);
+    assertFalse(FindXSS.search(userPost), "onclick attribute-breakout should be neutralised");
+  }
+
+  @Test
+  void legitimateHttpLink_stillRendersAndIsNotFlagged() {
+    String legit = "http://example.com/page?x=1";
+    String userPost = buildUserPost(legit);
+    assertTrue(
+        userPost.contains("http://example.com/page?x=1"), "legitimate link must survive unchanged");
+    assertFalse(FindXSS.search(userPost), "legitimate link must never be flagged as XSS");
+  }
+
+  @Test
+  void legitimateHttpsLink_stillWorks() {
+    String legit = "https://example.com/";
+    String userPost = buildUserPost(legit);
+    assertTrue(userPost.contains("https://example.com/"));
+    assertFalse(FindXSS.search(userPost));
+  }
+}