From b08505f610fb99397ca4b0d8e5948cf7ce9524b0 Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:46:06 -0700 Subject: [PATCH 1/2] Fix Challenge-33: enforce admin role check on UrlAccess2Admin Failure to Restrict URL Access 2's admin function (UrlAccess2Admin) only called Validate.validateSession(ses), which accepts any logged in user (player or admin). Any authenticated player who read the level's obfuscated client-side JS could discover the hidden admin endpoint and POST directly to it, receiving the privileged result key with no admin session required. Fix: add Validate.validateAdminSession(ses) as a gate at the top of doPost, returning 403 Forbidden before any challenge logic runs when the caller's session role is not 'admin'. Mirrors the same fix already applied and CI-verified for the sibling URL Access 1 challenge (UrlAccess1Admin, PR #330). UrlAccess2.java (the guest-view red herring servlet) needs no change, consistent with the working-list's parity note that the reference 40/40 PR only touches the *Admin.java files for these two challenges. Added src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java, a new integration test (same pattern as the existing NoSqlInjection1IT / XxeChallenge1IT tests) covering both cases end to end against a real MariaDB-backed instance of the actual servlet: a plain player session now gets 403 attempting the admin endpoint, and a real admin session still receives the correct result key. Locally verified: ran the new UrlAccess2AdminIT via mvn failsafe:integration-test/verify against a disposable MariaDB container (docker-compose bring-up was infeasible in this session due to heavy shared-WSL-sandbox contention from concurrent sibling agents, so this is a lighter but still live, real-code verification path). Both tests passed (2/2): the exploit path (non-admin hitting the admin servlet) now returns 403, and the legitimate admin flow still returns the key. Co-Authored-By: Claude Sonnet 5 --- .../module/challenge/UrlAccess2AdminIT.java | 99 +++++++++++++++++++ .../module/challenge/UrlAccess2Admin.java | 10 ++ 2 files changed, 109 insertions(+) create mode 100644 src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java diff --git a/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java b/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java new file mode 100644 index 000000000..02a26e99d --- /dev/null +++ b/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java @@ -0,0 +1,99 @@ +package servlets.module.challenge; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dbProcs.GetterIT; +import java.io.IOException; +import java.sql.SQLException; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletResponse; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockServletConfig; +import testUtils.TestProperties; + +/** + * Regression coverage for the "Failure to Restrict URL Access 2" challenge. The admin-only + * function must reject any session that is not actually holding the admin role, even when the + * caller already knows (or has guessed/reverse-engineered) the hidden request parameters the + * admin action expects. + */ +public class UrlAccess2AdminIT { + + private static final Logger log = LogManager.getLogger(UrlAccess2AdminIT.class); + private static final String LANG = "en_GB"; + private static final String applicationRoot = ""; + + private MockHttpServletRequest request; + private MockHttpServletResponse response; + + @BeforeAll + public static void resetDatabase() throws IOException, SQLException { + TestProperties.setTestPropertiesFileDirectory(log); + TestProperties.createMysqlResource(); + TestProperties.ensureSchemaReady(log); + TestProperties.reseedTestData(); + } + + @BeforeEach + public void setup() { + request = new MockHttpServletRequest(); + response = new MockHttpServletResponse(); + } + + private UrlAccess2Admin newServletInstance() throws ServletException { + UrlAccess2Admin servlet = new UrlAccess2Admin(); + servlet.init(new MockServletConfig("UrlAccess2Admin")); + return servlet; + } + + @Test + public void nonAdminPlayerIsForbiddenFromAdminFunction() throws Exception { + String userName = "urlAccess2Player"; + GetterIT.verifyTestUser(applicationRoot, userName, userName); + TestProperties.loginDoPost(log, request, response, userName, userName, null, LANG); + request.setCookies(response.getCookies()); + + // Fresh response for the challenge call: the mock response object still carries the 302 + // status set by the login redirect above, and the servlet under test never calls + // setStatus() on a success path (only sendError() on rejection), so reusing it would let a + // stale 302 mask a missing access-control check. + MockHttpServletResponse challengeResponse = new MockHttpServletResponse(); + request.addParameter("adminData", "youAreAnAdminOfAwesomenessWoopWoop"); + + newServletInstance().doPost(request, challengeResponse); + + assertEquals( + HttpServletResponse.SC_FORBIDDEN, + challengeResponse.getStatus(), + "A regular player must not be able to invoke the admin-only URL Access 2 function."); + } + + @Test + public void adminUserReceivesResultKey() throws Exception { + String userName = "urlAccess2Admin"; + GetterIT.verifyTestAdmin(applicationRoot, userName, userName); + TestProperties.loginDoPost(log, request, response, userName, userName, null, LANG); + request.setCookies(response.getCookies()); + + MockHttpServletResponse challengeResponse = new MockHttpServletResponse(); + request.addParameter("adminData", "youAreAnAdminOfAwesomenessWoopWoop"); + + newServletInstance().doPost(request, challengeResponse); + + assertEquals( + HttpServletResponse.SC_OK, + challengeResponse.getStatus(), + "A real admin must still be able to complete the legitimate admin function."); + String body = challengeResponse.getContentAsString(); + assertTrue( + body != null && !body.isEmpty() && !body.contains("failue"), + "Admin response should contain the result key output, not the failure branch."); + } +} diff --git a/src/main/java/servlets/module/challenge/UrlAccess2Admin.java b/src/main/java/servlets/module/challenge/UrlAccess2Admin.java index 0614491a5..605f464af 100644 --- a/src/main/java/servlets/module/challenge/UrlAccess2Admin.java +++ b/src/main/java/servlets/module/challenge/UrlAccess2Admin.java @@ -65,6 +65,16 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) ses.getAttribute("userName").toString()); log.debug(levelName + " servlet accessed by: " + ses.getAttribute("userName").toString()); PrintWriter out = response.getWriter(); + + if (!Validate.validateAdminSession(ses)) { + log.fatal( + levelName + + " admin-only servlet accessed by non-admin user: " + + ses.getAttribute("userName").toString()); + response.sendError(HttpServletResponse.SC_FORBIDDEN); + return; + } + out.print(getServletInfo()); String htmlOutput = new String(); From ce9f8346823906a1526e30ff9cea1e6847d34a1e Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:52:32 -0700 Subject: [PATCH 2/2] Reformat UrlAccess2AdminIT javadoc to satisfy spotless (google-java-format) CI's spotless-check failed on the class javadoc's line wrapping. Ran mvn spotless:apply locally and confirmed mvn spotless:check now passes clean. Co-Authored-By: Claude Sonnet 5 --- .../java/servlets/module/challenge/UrlAccess2AdminIT.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java b/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java index 02a26e99d..611e93e35 100644 --- a/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java +++ b/src/it/java/servlets/module/challenge/UrlAccess2AdminIT.java @@ -19,10 +19,9 @@ import testUtils.TestProperties; /** - * Regression coverage for the "Failure to Restrict URL Access 2" challenge. The admin-only - * function must reject any session that is not actually holding the admin role, even when the - * caller already knows (or has guessed/reverse-engineered) the hidden request parameters the - * admin action expects. + * Regression coverage for the "Failure to Restrict URL Access 2" challenge. The admin-only function + * must reject any session that is not actually holding the admin role, even when the caller already + * knows (or has guessed/reverse-engineered) the hidden request parameters the admin action expects. */ public class UrlAccess2AdminIT {