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();
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);