From 929fff3e34bfd7bae58aee3f1addc7d1f7d9e8f1 Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:32:35 -0700 Subject: [PATCH 1/3] fix: harden XssFilter.badUrlValidate() against href attribute-injection (XSS Challenge 5) badUrlValidate() spliced its whitelisted 'URL' straight into a double-quoted href="" attribute in XssChallengeFive, but only HTML-encoded the FIRST double quote it found (replaceFirst) while every other dangerous character (#, <, >) was correctly replaceAll'd. A payload with one harmless decoy quote up front left a later, real quote un-encoded, letting an attacker close the href attribute early and splice in a brand new onmouseover/ onerror/onclick attribute - HTML parsers accept a new attribute immediately after a closing quote with no whitespace required. Fix: replaceAll instead of replaceFirst so every quote is neutralised, plus a defence-in-depth check that the parsed URL's protocol is actually http/ https before it is ever returned, in case URL parsing/normalisation could otherwise smuggle another scheme through. Verified locally against the real compiled classes (outside the servlet container): the pre-fix code lets payloads like http:"foo"onmouseover="alert(1) produce ... which FindXSS.search() flags as XSS; the fixed code HTML-encodes every quote so the same payloads are no longer detected as XSS, while legitimate http(s) links are unaffected and still render as a clickable link. Co-Authored-By: Claude Sonnet 5 --- src/main/java/utils/XssFilter.java | 16 ++++- .../utils/XssChallengeFiveExploitTest.java | 66 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 src/test/java/utils/XssChallengeFiveExploitTest.java 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..0423baf4f --- /dev/null +++ b/src/test/java/utils/XssChallengeFiveExploitTest.java @@ -0,0 +1,66 @@ +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 = "Your HTTP Link!") + * 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 + * brand new onmouseover/onerror/onclick attribute that HTML parsers (browsers and the + * JTidy/Jsoup pipeline FindXSS.search() uses) accept even with no whitespace between + * attributes. Confirmed against the original code (outside this test, via a standalone + * harness) that this exact payload shape scores FindXSS.search() == true; these tests + * assert the fixed code closes it while leaving legitimate http(s) links untouched. + */ +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)); + } +} From 56fedfcb238ba933ef0e5e79e43b7b97847c9cd5 Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:36:41 -0700 Subject: [PATCH 2/3] style: apply spotless/google-java-format to XssChallengeFiveExploitTest Co-Authored-By: Claude Sonnet 5 --- .../utils/XssChallengeFiveExploitTest.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/test/java/utils/XssChallengeFiveExploitTest.java b/src/test/java/utils/XssChallengeFiveExploitTest.java index 0423baf4f..eeb367501 100644 --- a/src/test/java/utils/XssChallengeFiveExploitTest.java +++ b/src/test/java/utils/XssChallengeFiveExploitTest.java @@ -6,19 +6,19 @@ import org.junit.jupiter.api.Test; /** - * Reproduces the XssChallengeFive.doPost() output-construction chain - * (userPost = "Your HTTP Link!") - * without needing the full servlet container. + * Reproduces the XssChallengeFive.doPost() output-construction chain (userPost = "Your HTTP Link!") 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 - * brand new onmouseover/onerror/onclick attribute that HTML parsers (browsers and the - * JTidy/Jsoup pipeline FindXSS.search() uses) accept even with no whitespace between - * attributes. Confirmed against the original code (outside this test, via a standalone - * harness) that this exact payload shape scores FindXSS.search() == true; these tests - * assert the fixed code closes it while leaving legitimate http(s) links untouched. + * (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 brand new + * onmouseover/onerror/onclick attribute that HTML parsers (browsers and the JTidy/Jsoup pipeline + * FindXSS.search() uses) accept even with no whitespace between attributes. Confirmed against the + * original code (outside this test, via a standalone harness) that this exact payload shape + * scores FindXSS.search() == true; these tests assert the fixed code closes it while leaving + * legitimate http(s) links untouched. */ class XssChallengeFiveExploitTest { @@ -52,7 +52,8 @@ void decoyQuoteOnclickBreakout_noLongerDetectedAsXss() { 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"); + 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"); } From 6833b81855170491ba5009708d72c9f43de811ba Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:40:47 -0700 Subject: [PATCH 3/3] style: switch test file's class comment to // lines to avoid javadoc reflow mismatch Co-Authored-By: Claude Sonnet 5 --- .../utils/XssChallengeFiveExploitTest.java | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/test/java/utils/XssChallengeFiveExploitTest.java b/src/test/java/utils/XssChallengeFiveExploitTest.java index eeb367501..571543942 100644 --- a/src/test/java/utils/XssChallengeFiveExploitTest.java +++ b/src/test/java/utils/XssChallengeFiveExploitTest.java @@ -5,21 +5,19 @@ import org.junit.jupiter.api.Test; -/** - * Reproduces the XssChallengeFive.doPost() output-construction chain (userPost = "Your HTTP Link!") 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 brand new - * onmouseover/onerror/onclick attribute that HTML parsers (browsers and the JTidy/Jsoup pipeline - * FindXSS.search() uses) accept even with no whitespace between attributes. Confirmed against the - * original code (outside this test, via a standalone harness) that this exact payload shape - * scores FindXSS.search() == true; these tests assert the fixed code closes it while leaving - * legitimate http(s) links untouched. - */ +// 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) {