diff --git a/src/main/java/utils/XssFilter.java b/src/main/java/utils/XssFilter.java index ad1c27d58..9d34f2def 100644 --- a/src/main/java/utils/XssFilter.java +++ b/src/main/java/utils/XssFilter.java @@ -77,8 +77,20 @@ public static String badUrlValidate(String input) { .replaceAll("#", "#") .replaceAll("<", "<") .replaceAll(">", ">") - .replaceFirst("\"", """)); - 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 tag. + .replaceAll("\"", """)); + // 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; diff --git a/src/test/java/utils/XssChallengeFiveExploitTest.java b/src/test/java/utils/XssChallengeFiveExploitTest.java new file mode 100644 index 000000000..571543942 --- /dev/null +++ b/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 +// "Your HTTP Link!", 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 "Your HTTP Link!"; + } + + @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)); + } +}