From 9a4f22b7bf2c2da0eae3092e15dc98e2cd0b7350 Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:48:28 -0700 Subject: [PATCH] Fix Session Management 5: forgeable password-reset token SessionManagement5SetToken claimed to email a reset token but never generated or stored one; SessionManagement5ChangePassword accepted any base64'd timestamp within the last 10 minutes as valid, so an attacker could forge a token for any user (including admin) purely from their own clock, with no server interaction required. - SessionManagement5SetToken now issues a real SecureRandom token per user and records it server-side with an issue time. - SessionManagement5ChangePassword now requires that exact token (constant-time compared), keeps the 10-minute freshness window, and consumes the token on success or expiry (single use). - SessionManagement5 now verifies the password for every login attempt before trusting the looked-up role, instead of only when the role lookup already resolved to admin. Co-Authored-By: Claude Sonnet 5 --- .../module/challenge/SessionManagement5.java | 67 +++++++-------- .../SessionManagement5ChangePassword.java | 82 ++++++++----------- .../challenge/SessionManagement5SetToken.java | 46 ++++++++++- 3 files changed, 104 insertions(+), 91 deletions(-) diff --git a/src/main/java/servlets/module/challenge/SessionManagement5.java b/src/main/java/servlets/module/challenge/SessionManagement5.java index 0de15b90e..6cdeb9422 100644 --- a/src/main/java/servlets/module/challenge/SessionManagement5.java +++ b/src/main/java/servlets/module/challenge/SessionManagement5.java @@ -107,50 +107,38 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) callstmt.execute(); log.debug("Changes committed."); - callstmt = conn.prepareStatement("SELECT userName, userRole FROM users WHERE userName = ?"); + // The password must be verified up front, for every account, before any role or identity + // information is trusted. The previous version only checked the password once a role + // lookup had already returned "admin" - any other role fell straight into a signed-in + // session without a password ever being verified. + callstmt = + conn.prepareStatement( + "SELECT userName, userRole FROM users WHERE userName = ? AND userPassword =" + + " SHA(?)"); callstmt.setString(1, subName); - log.debug("Executing findUser"); + callstmt.setString(2, subPass); + log.debug("Executing Login Check"); ResultSet resultSet = callstmt.executeQuery(); - // Is the username valid? if (resultSet.next()) { - log.debug("User found"); - // Is the user an Admin? + log.debug("Credentials verified"); if (resultSet.getString(2).equalsIgnoreCase("admin")) { - log.debug("Admin Detected"); - callstmt = - conn.prepareStatement( - "SELECT userName, userRole FROM users WHERE userName = ? AND userPassword =" - + " SHA(?)"); - callstmt.setString(1, subName); - callstmt.setString(2, subPass); - log.debug("Executing Login Check"); - ResultSet resultSet2 = callstmt.executeQuery(); - if (resultSet2.next()) { - log.debug("Successful Admin Login"); - // Get key and add it to the output - String userKey = - Hash.generateUserSolution(levelResult, (String) ses.getAttribute("userName")); + log.debug("Successful Admin Login"); + // Get key and add it to the output + String userKey = + Hash.generateUserSolution(levelResult, (String) ses.getAttribute("userName")); - htmlOutput = - "

" - + bundle.getString("response.welcome") - + " " - + Encode.forHtml(resultSet2.getString(1)) - + "

" - + "

" - + bundle.getString("response.resultKey") - + " " - + userKey - + "" - + "

"; - } else { - userAddress = - bundle.getString("response.badPass") - + " " - + Encode.forHtml(resultSet.getString(1)) - + "
"; - htmlOutput = makeTable(userAddress, bundle); - } + htmlOutput = + "

" + + bundle.getString("response.welcome") + + " " + + Encode.forHtml(resultSet.getString(1)) + + "

" + + "

" + + bundle.getString("response.resultKey") + + " " + + userKey + + "" + + "

"; } else { log.debug("Successful Pleb Login"); htmlOutput = @@ -163,6 +151,7 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) + "



"; } } else { + log.debug("Invalid username or password"); userAddress = bundle.getString("response.badUser") + "
"; htmlOutput = makeTable(userAddress, bundle); } diff --git a/src/main/java/servlets/module/challenge/SessionManagement5ChangePassword.java b/src/main/java/servlets/module/challenge/SessionManagement5ChangePassword.java index a48ddc69e..5df382e1b 100644 --- a/src/main/java/servlets/module/challenge/SessionManagement5ChangePassword.java +++ b/src/main/java/servlets/module/challenge/SessionManagement5ChangePassword.java @@ -3,12 +3,9 @@ import dbProcs.Database; import java.io.IOException; import java.io.PrintWriter; -import java.io.UnsupportedEncodingException; +import java.security.MessageDigest; import java.sql.Connection; import java.sql.PreparedStatement; -import java.text.ParseException; -import java.text.SimpleDateFormat; -import java.util.Date; import java.util.Locale; import java.util.ResourceBundle; import javax.servlet.ServletException; @@ -16,7 +13,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import org.apache.commons.codec.binary.Base64; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import utils.ShepherdLogManager; @@ -51,12 +47,13 @@ public class SessionManagement5ChangePassword extends HttpServlet { /** * Function used by Session Management Challenge Five to change the password of the submitted user - * name. The function requires a valid token which is a base64'd timestamp. If the current time is - * within 10 minutes of the token, the function will execute + * name. The function requires the exact, unguessable token that was issued to that same user name + * by {@link SessionManagement5SetToken}. If the token matches and is still within 10 minutes of + * being issued, the function will execute and the token is consumed (single use). * * @param userName User cookie used to store the user password to be reset * @param newPassword the password which to use to update an accounts password - * @param resetPasswordToken Base64'd time stamp + * @param resetPasswordToken The token previously issued for this exact user name */ public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { @@ -80,8 +77,6 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) PrintWriter out = response.getWriter(); out.print(getServletInfo()); String htmlOutput = new String(); - String errorMessage = new String(); - int tokenLife = 11; try { log.debug("Getting Challenge Parameters"); Object passNewObj = request.getParameter("newPassword"); @@ -102,33 +97,20 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) log.debug("userName = " + userName); log.debug("newPass = " + newPass); log.debug("token = " + token); - String tokenTime = new String(); - try { - byte[] decodedToken = Base64.decodeBase64(token); - tokenTime = new String(decodedToken, "UTF-8"); - } catch (UnsupportedEncodingException e) { - log.debug("Could not decode password token"); - errorMessage += "

" + bundle.getString("changePass.noDecode") + "

"; - } - if (tokenTime.isEmpty()) { - log.debug("Could not decode token. Ending Servlet."); - out.write(errorMessage); + + // The token is only ever valid if it is the exact, unguessable value this application + // itself generated and handed out for THIS user name via SessionManagement5SetToken. + // Unlike a self-describing timestamp, nothing here can be derived or forged purely from + // the current time - the reset request has to have genuinely happened first. + SessionManagement5SetToken.TokenRecord issuedToken = + SessionManagement5SetToken.RESET_TOKENS.get(userName); + + if (issuedToken == null || token.isEmpty() || !tokensMatch(token, issuedToken.token)) { + log.debug("No matching outstanding reset token for user: " + userName); + htmlOutput = "

" + bundle.getString("changePass.funkyToken") + "

"; } else { - log.debug("Decoded Token = " + tokenTime); - - // Get Time from Token and see if it is inside the last 10 minutes - SimpleDateFormat simpleDateFormat = new SimpleDateFormat("EEE MMM d HH:mm:ss Z yyyy"); - try { - Date tokenDateTime = simpleDateFormat.parse(tokenTime); - Date currentDateTime = new Date(); - // Get difference in minutes - tokenLife = - (int) ((currentDateTime.getTime() / 60000) - (tokenDateTime.getTime() / 60000)); - log.debug("Token life = " + tokenLife); - } catch (ParseException e) { - log.error("Date Parsing Error: " + e.toString()); - errorMessage += bundle.getString("changePass.badTokenData") + ": " + e.toString(); - } + long tokenLife = (System.currentTimeMillis() - issuedToken.issuedAtMillis) / 60000; + log.debug("Token life (minutes) = " + tokenLife); if (tokenLife < 10 && tokenLife >= 0) { if (newPass.length() >= 12) { @@ -157,24 +139,21 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) callstmt.execute(); log.debug("Changes committed."); + // Single use - the token must not be replayable once it has been redeemed. + SessionManagement5SetToken.RESET_TOKENS.remove(userName); + htmlOutput = "

" + bundle.getString("changePass.success") + "

"; } else { log.debug("Invalid password submitted: " + newPass); htmlOutput = "

" + bundle.getString("changePass.failure") + "

"; } + } else if (tokenLife >= 10) { + log.debug("Token too old"); + SessionManagement5SetToken.RESET_TOKENS.remove(userName); + htmlOutput = "

" + bundle.getString("changePass.oldToken") + "

"; } else { - if (!errorMessage.isEmpty()) { - htmlOutput = "

" + errorMessage + ""; - } else if (tokenLife >= 10) { - log.debug("Token too old"); - htmlOutput = "

" + bundle.getString("changePass.oldToken") + "

"; - } else if (tokenLife < 0) { - log.debug("Token to young"); - htmlOutput = "

" + bundle.getString("changePass.youngToken") + "

"; - } else { - log.error("Token to Strange: Unexpected Error"); - htmlOutput = "

" + bundle.getString("changePass.funkyToken") + "

"; - } + log.error("Token life negative: Unexpected Error"); + htmlOutput = "

" + bundle.getString("changePass.funkyToken") + "

"; } } log.debug("Outputting HTML"); @@ -187,4 +166,11 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) log.error(levelName + " servlet accessed with no session"); } } + + /** Constant-time comparison so token validation timing cannot leak information about it. */ + private static boolean tokensMatch(String submitted, String issued) { + return MessageDigest.isEqual( + submitted.getBytes(java.nio.charset.StandardCharsets.UTF_8), + issued.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + } } diff --git a/src/main/java/servlets/module/challenge/SessionManagement5SetToken.java b/src/main/java/servlets/module/challenge/SessionManagement5SetToken.java index 7080e18ca..fb9f8fc7b 100644 --- a/src/main/java/servlets/module/challenge/SessionManagement5SetToken.java +++ b/src/main/java/servlets/module/challenge/SessionManagement5SetToken.java @@ -3,16 +3,20 @@ import dbProcs.Database; import java.io.IOException; import java.io.PrintWriter; +import java.security.SecureRandom; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.util.Locale; +import java.util.Map; import java.util.ResourceBundle; +import java.util.concurrent.ConcurrentHashMap; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import org.apache.commons.codec.binary.Base64; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.owasp.encoder.Encode; @@ -22,10 +26,12 @@ /** * Session Management Challenge Five SessionManagement5SetToken (Does not Return Result Key) * - *

This function is a shell to give the appearance that a token has been set for a user. A DB - * call is made to check if a user exists. If the user does exist the server returns an ok message - * claiming that the user has been emailed a URL with a token embedded for resetting their password. - * This in fact does not happen. User must find another way to sign in as an admin. + *

A DB call is made to check if a user exists. If the user does exist, a fresh unguessable reset + * token is generated and recorded server-side for that exact user name, and the server returns an + * ok message claiming that the user has been emailed a URL with the token embedded for resetting + * their password (the outgoing email itself is out of scope for this challenge). Only the holder of + * that exact token - i.e. whoever actually receives the email for that account - can subsequently + * use {@link SessionManagement5ChangePassword} to reset the password. * *


*
@@ -50,6 +56,32 @@ public class SessionManagement5SetToken extends HttpServlet { private static final Logger log = LogManager.getLogger(SessionManagement5SetToken.class); private static String levelName = "SessionManagement5SetToken"; public static String levelHash = SessionManagement5.levelHash; + private static final SecureRandom RANDOM_GENERATOR = new SecureRandom(); + + /** + * Holds the single valid password reset token currently outstanding for a given user name. A + * record is only ever created here, by this servlet, in response to a genuine reset request for + * that exact user name - it is never derived from or predictable using client supplied data such + * as the current time. + */ + static final class TokenRecord { + final String token; + final long issuedAtMillis; + + TokenRecord(String token, long issuedAtMillis) { + this.token = token; + this.issuedAtMillis = issuedAtMillis; + } + } + + /** userName -> the one outstanding reset token issued for that user. */ + static final Map RESET_TOKENS = new ConcurrentHashMap<>(); + + private static String generateToken() { + byte[] randomBytes = new byte[32]; + RANDOM_GENERATOR.nextBytes(randomBytes); + return Base64.encodeBase64URLSafeString(randomBytes); + } /** * Used to apparently send a message to a user with a token to reset their password. @@ -110,6 +142,12 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) // Is the username valid? if (resultSet.next()) { log.debug("User found"); + // Issue a fresh, unguessable reset token for this exact user and remember it + // server-side so it can later be validated. In a real deployment this token (never the + // time it was issued) would be the value emailed to the account owner. + String resetToken = generateToken(); + RESET_TOKENS.put(userName, new TokenRecord(resetToken, System.currentTimeMillis())); + log.debug("Issued password reset token for '" + userName + "': " + resetToken); htmlOutput = bundle.getString("setToken.sentTo.1") + " '"