From 4372657a5310b4badd430940c44513c59d381a80 Mon Sep 17 00:00:00 2001 From: JBHook <314778749+JBHook@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:07:19 +0000 Subject: [PATCH 1/2] Fix weak CSRF nonce in CSRF Challenge Six Target The nonce was random.nextInt(3) used as an index into a 3-element array, so only 3 possible values ever existed - an attacker could brute-force a valid nonce in at most 3 tries without ever touching the victim's session. Switched to utils.Hash.randomString(), matching the real-random pattern already used correctly in CsrfChallengeTargetFour/Seven. Co-Authored-By: Claude Sonnet 5 --- .../module/challenge/CsrfChallengeTargetSix.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/main/java/servlets/module/challenge/CsrfChallengeTargetSix.java b/src/main/java/servlets/module/challenge/CsrfChallengeTargetSix.java index 684fd90b2..d328fa901 100644 --- a/src/main/java/servlets/module/challenge/CsrfChallengeTargetSix.java +++ b/src/main/java/servlets/module/challenge/CsrfChallengeTargetSix.java @@ -5,7 +5,6 @@ import java.io.IOException; import java.io.PrintWriter; import java.util.Locale; -import java.util.Random; import java.util.ResourceBundle; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -14,6 +13,7 @@ import javax.servlet.http.HttpSession; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import utils.Hash; import utils.ShepherdLogManager; import utils.Validate; @@ -42,11 +42,6 @@ public class CsrfChallengeTargetSix extends HttpServlet { private static final long serialVersionUID = 1L; private static String moduleHash = "2fff41105149e507c75b5a54e558470469d7024929cf78d570cd16c03bee3569"; - private static final String[] csrfArray = { - "c4ca4238a0b923820dcc509a6f75849b", - "c81e728d9d4c2f636f067f89cc14862c", - "eccbc87e4b5ce2fe28308fd9f2a7baf3" - }; private static final Logger log = LogManager.getLogger(CsrfChallengeTargetSix.class); private static String levelName = "CSRF 6 Target"; @@ -85,9 +80,11 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) if (ses.getAttribute(csrfTokenName) == null || ses.getAttribute(csrfTokenName).toString().isEmpty()) { log.debug("No CSRF Token associated with user"); - Random random = new Random(); - int newToken = random.nextInt(3); - storedToken = csrfArray[newToken]; + // random.nextInt(3) as an index into a 3-element array only ever produces one of 3 + // possible values, letting an attacker brute-force a valid nonce in at most 3 tries + // without ever touching the victim's session. Hash.randomString() matches the + // real-random pattern used elsewhere. + storedToken = Hash.randomString(); out.write( csrfGenerics.getString("target.noTokenNewToken") + " " + storedToken + "

"); ses.setAttribute(csrfTokenName, storedToken); From 47cef74144f5f7215d1fdc73ec7346f5c8c8276a Mon Sep 17 00:00:00 2001 From: JBHook <314778749+JBHook@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:57:09 +0000 Subject: [PATCH 2/2] Fix IDOR in CSRF Challenge Six's token-retrieval endpoint The weak-nonce fix in this branch only addressed CsrfChallengeTargetSix's own token generation (random.nextInt(3) -> Hash.randomString()), but left CsrfChallengeSixGetToken untouched: it took the userId to look up from a request parameter instead of the caller's own session, and matched it with LIKE (wildcard metacharacters unescaped) instead of an exact match - letting any user fetch any other user's real CSRF token directly, completely defeating the point of the token being unpredictable. Same bug class, same fix, as the equivalent endpoint already fixed for CSRF Challenge Seven. Scoped the lookup to the caller's own session userId (ses.getAttribute("userStamp")) and switched LIKE to an exact match. --- .../module/challenge/CsrfChallengeSixGetToken.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/servlets/module/challenge/CsrfChallengeSixGetToken.java b/src/main/java/servlets/module/challenge/CsrfChallengeSixGetToken.java index cb61a3d85..716e52d35 100644 --- a/src/main/java/servlets/module/challenge/CsrfChallengeSixGetToken.java +++ b/src/main/java/servlets/module/challenge/CsrfChallengeSixGetToken.java @@ -69,7 +69,12 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) if (Validate.validateSession(ses)) { log.debug(levelName + " servlet accessed by: " + ses.getAttribute("userName").toString()); String htmlOutput = new String("Your csrf Token for this Challenge is: "); - String userId = request.getParameter("userId").toString(); + // The userId must be the caller's own - taking it from a request parameter let any + // authenticated user fetch any other user's real CSRF token directly, and the LIKE + // match (with wildcard metacharacters left unescaped) made it worse by allowing broad + // token disclosure. This defeats the point of the token being unguessable, since the + // whole defense relies on an attacker being unable to learn the victim's value. + String userId = (String) ses.getAttribute("userStamp"); Connection conn = Database.getChallengeConnection( @@ -78,7 +83,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) log.debug("Preparing setCsrfChallengeSixToken call"); PreparedStatement callstmnt = conn.prepareStatement( - "SELECT csrfTokenscol FROM csrfchallengesix.csrfTokens WHERE userId LIKE ?"); + "SELECT csrfTokenscol FROM csrfchallengesix.csrfTokens WHERE userId = ?"); callstmnt.setString(1, userId); log.debug("Executing setCsrfChallengeSixTokenQuery"); ResultSet rs = callstmnt.executeQuery();