diff --git a/src/main/java/servlets/module/challenge/UrlAccess3.java b/src/main/java/servlets/module/challenge/UrlAccess3.java index 0c13de19d..496e39df0 100644 --- a/src/main/java/servlets/module/challenge/UrlAccess3.java +++ b/src/main/java/servlets/module/challenge/UrlAccess3.java @@ -1,6 +1,5 @@ package servlets.module.challenge; -import dbProcs.Getter; import java.io.IOException; import java.io.PrintWriter; import java.util.Locale; @@ -14,7 +13,6 @@ import org.apache.commons.codec.binary.Base64; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import utils.Hash; import utils.ShepherdLogManager; import utils.Validate; @@ -41,18 +39,20 @@ public class UrlAccess3 extends HttpServlet { private static final long serialVersionUID = 1L; private static final Logger log = LogManager.getLogger(UrlAccess3.class); private static String levelName = "Failure to Restrict URL Access 3"; - private static String levelHash = - "e40333fc2c40b8e0169e433366350f55c77b82878329570efa894838980de5b4"; /** - * Users must take advance of the broken session management in this application by modifying the - * tracking cookie "currentPerson" which is encoded in Base64. They must modify this cookie to be - * equal a super admin to access the result key. + * The "currentPerson" cookie is nothing more than client-supplied, unsigned state: the browser + * sets it with plain JavaScript and nothing on the server ever issues, signs, or otherwise + * vouches for its value. Because of that it must never be trusted to answer an authorization + * question - not even when it happens to spell out a privileged-sounding name - so this servlet + * no longer branches on its content at all. There is no legitimate, server-verified way for a + * request to actually be the sub-schema's super admin, so that response is simply unreachable now + * rather than gated behind an easily-forged claim. * * @param userId Red herring that is pre set to d3d9446802a44259755d38e6d163e820 * @param secure Red herring that is pre set to true * @param adminDetected Red herring - * @param currentPerson Cookie encoded base64 that manages who is signed in to the sub schema + * @param currentPerson Untrusted, client-controlled cookie retained only for logging */ public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { @@ -94,26 +94,16 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) String decodedCookie = new String(decodedCookieBytes, "UTF-8"); log.debug("Decoded Cookie: " + decodedCookie); - if (decodedCookie.equals("MrJohnReillyTheSecond")) { - log.debug("Super Admin Cookie detected"); - // Get key and add it to the output - String userKey = - Hash.generateUserSolution( - Getter.getModuleResultFromHash(getServletContext().getRealPath(""), levelHash), - (String) ses.getAttribute("userName")); - htmlOutput = - "

" - + bundle.getString("admin.superAdminClub") - + "

" - + "

" - + bundle.getString("admin.superAdminClub.keyMessage") - + " " - + "" - + userKey - + "" - + "

"; - } else if (!decodedCookie.equals("aGuest")) { - log.debug("Tampered role cookie detected: " + decodedCookie); + if (!decodedCookie.equals("aGuest")) { + // Whatever the cookie claims - including the super admin's name - it is only ever + // an unverified assertion from the client. It is logged for visibility but it can + // never unlock anything beyond the plain guest view. + log.fatal( + "User " + + ses.getAttribute("userName") + + " submitted a forged currentPerson cookie claiming to be '" + + decodedCookie + + "'; ignoring it, no privileged view exists to grant."); htmlOutput = ""; } else { log.debug("No change to role cookie submitted"); diff --git a/src/main/java/servlets/module/challenge/UrlAccess3UserList.java b/src/main/java/servlets/module/challenge/UrlAccess3UserList.java index 1a0e0fcee..03a981805 100644 --- a/src/main/java/servlets/module/challenge/UrlAccess3UserList.java +++ b/src/main/java/servlets/module/challenge/UrlAccess3UserList.java @@ -9,12 +9,10 @@ import java.util.Locale; import java.util.ResourceBundle; import javax.servlet.ServletException; -import javax.servlet.http.Cookie; 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; @@ -70,39 +68,25 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) String htmlOutput = new String(); try { - Cookie userCookies[] = request.getCookies(); - int i = 0; - Cookie theCookie = null; - for (i = 0; i < userCookies.length; i++) { - if (userCookies[i].getName().compareTo("currentPerson") == 0) { - theCookie = userCookies[i]; - break; // End Loop, because we found the token - } - } - String currentUser = new String("aGuest"); - if (theCookie != null) { - log.debug("Cookie value: " + theCookie.getValue()); - byte[] decodedCookieBytes = Base64.decodeBase64(theCookie.getValue()); - String decodedCookie = new String(decodedCookieBytes, "UTF-8"); - log.debug("Decoded Cookie: " + decodedCookie); - currentUser = decodedCookie; - } + // This directory lookup used to take its identity from the client-supplied + // "currentPerson" cookie and concatenate it straight into the SQL text, so anyone could + // both inject arbitrary SQL through the cookie and, even without injecting anything, + // enumerate every "admin" account in the sub-schema (including hints toward the real + // super admin) purely by asking. A request to this endpoint has no legitimate reason to + // see anyone's row but the fixed public guest entry, so the lookup identity below is a + // hardcoded constant - never derived from request/cookie input - and the query no longer + // has a clause that discloses privileged accounts at all. + final String publicDirectoryEntry = "aGuest"; String ApplicationRoot = getServletContext().getRealPath(""); Connection conn = Database.getChallengeConnection(ApplicationRoot, "UrlAccessThree"); PreparedStatement callstmt; - callstmt = - conn.prepareStatement( - "SELECT userName FROM users WHERE userRole = \"admin\" OR userName = \"" - + currentUser - + "\";"); + callstmt = conn.prepareStatement("SELECT userName FROM users WHERE userName = ?;"); + callstmt.setString(1, publicDirectoryEntry); log.debug("Getting User List"); htmlOutput = new String(); ResultSet rs = callstmt.executeQuery(); while (rs.next()) { htmlOutput += Encode.forHtml(rs.getString(1)) + "
"; - if (rs.getString(1).equalsIgnoreCase("MrJohnReillyTheSecond")) { - log.debug("Super Admin contained in response"); - } } } catch (Exception e) { htmlOutput = new String(errors.getString("error.funky"));