diff --git a/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java index d23f59e2e..ac5225d5d 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerability.java @@ -1,6 +1,5 @@ package org.sasanlabs.service.vulnerability.idor; -import java.util.Base64; import java.util.List; import org.sasanlabs.internal.utility.LevelConstants; import org.sasanlabs.internal.utility.Variant; @@ -77,15 +76,19 @@ public ResponseEntity> level1( String actualToken = cookieToken; try { if (actualToken != null) { - idorLoginService.decodeToken(actualToken); - if (id != null) { - User profile = fetchUserById(id); - if (profile == null) { - return response(USER_NOT_FOUND, false); - } - return response(profile, true); + User decodedUser = idorLoginService.decodeToken(actualToken); + int tokenUserId = decodedUser.getUserId(); + int requestedId = id != null ? id : tokenUserId; + + if (requestedId != tokenUserId) { + return response(ACCESS_DENIED_INSUFFICIENT, false); + } + + User profile = fetchUserById(requestedId); + if (profile == null) { + return response(USER_NOT_FOUND, false); } - return response(USER_NOT_FOUND, false); + return response(profile, true); } return response(PROVIDE_LOGIN_OR_TOKEN, false); @@ -113,11 +116,15 @@ public ResponseEntity> level2( @CookieValue(value = COOKIE_TOKEN_LEVEL_2, required = false) String cookieToken, @CookieValue(value = COOKIE_USER_ID_LEVEL_2, required = false) Integer loggedInUser) { + // Note: loggedInUser is an unsigned, attacker-controllable cookie left in the request + // model for backward compatibility with the login flow, but the user identity used for + // the lookup below is always taken from the signed token, never from this cookie - + // otherwise an attacker could simply edit userId_level2 to view another user's profile. String actualToken = cookieToken; try { - if (actualToken != null && loggedInUser != null) { - idorLoginService.decodeToken(actualToken); - User profile = fetchUserById(loggedInUser); + if (actualToken != null) { + User decodedUser = idorLoginService.decodeToken(actualToken); + User profile = fetchUserById(decodedUser.getUserId()); if (profile == null) { return response(USER_NOT_FOUND, false); } @@ -150,12 +157,17 @@ public ResponseEntity> level3( @CookieValue(value = COOKIE_ROLE_LEVEL_3, required = false) String cookieRole, @RequestParam(required = false) Integer id) { + // Note: cookieRole is an unsigned, attacker-controllable cookie. It is accepted as a + // request parameter for backward compatibility but must never be trusted as the + // authoritative role - only the role embedded in the signed JWT is used for the + // authorization decision below, otherwise an attacker could set role_level3=ADMIN to + // impersonate an administrator. String actualToken = cookieToken; try { if (actualToken != null) { User decodedUser = idorLoginService.decodeToken(actualToken); int tokenUserId = decodedUser.getUserId(); - String role = cookieRole != null ? cookieRole : decodedUser.getRole(); + String role = decodedUser.getRole(); if (id == null) { id = tokenUserId; @@ -199,12 +211,16 @@ public ResponseEntity> level4( @CookieValue(value = COOKIE_ROLE_LEVEL_4, required = false) String cookieRole, @RequestParam(required = false) Integer id) { + // Note: cookieRole here is merely base64-encoded, not signed - encoding is not + // encryption/authentication, so it is just as forgeable as the plaintext cookie in + // level 3. As with level 3, the role used for the authorization decision below always + // comes from the signed JWT, never from this cookie. String actualToken = cookieToken; try { if (actualToken != null) { User decodedUser = idorLoginService.decodeToken(actualToken); int tokenUserId = decodedUser.getUserId(); - String role = cookieRole != null ? decodeBase64(cookieRole) : decodedUser.getRole(); + String role = decodedUser.getRole(); if (id == null) { id = tokenUserId; @@ -304,14 +320,6 @@ private List fetchAllUsers() { rs.getString("role"))); } - private String decodeBase64(String encodedId) { - try { - return new String(Base64.getUrlDecoder().decode(encodedId)); - } catch (IllegalArgumentException e) { - return null; - } - } - private ResponseEntity> response( Object content, boolean isValid) { return new ResponseEntity<>( diff --git a/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java index 377dc9fd6..56cbb31d4 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerability.java @@ -92,6 +92,13 @@ private ResponseEntity> getJWTResponseB genericVulnerabilityResponseBean, headers, HttpStatus.OK); } + // Previously this endpoint accepted the JWT to be verified as a "JWT" query parameter + // (?JWT=...), which means the token ends up in the URL: it gets written to server access + // logs, browser history, the Referer header of any subsequent cross-origin request, and any + // proxy/CDN logs along the way. A bearer token is a credential and must never travel in the + // URL; it belongs in a header (or the request body) instead, so it now reads the token from + // the standard Authorization header, matching the pattern already used by the other + // header-based levels in this class. @AttackVector( vulnerabilityExposed = VulnerabilityType.CLIENT_SIDE_VULNERABLE_JWT, description = "JWT_URL_EXPOSING_SECURE_INFORMATION") @@ -99,13 +106,18 @@ private ResponseEntity> getJWTResponseB value = LevelConstants.LEVEL_1, htmlTemplate = "LEVEL_1/JWT_Level1") public ResponseEntity> - getVulnerablePayloadLevelUnsecure(@RequestParam Map queryParams) + getVulnerablePayloadLevelUnsecure(RequestEntity requestEntity) throws UnsupportedEncodingException, ServiceApplicationException { Optional symmetricAlgorithmKey = jwtAlgorithmKMS.getSymmetricAlgorithmKey( JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, KeyStrength.HIGH); LOGGER.info(symmetricAlgorithmKey.isPresent() + " " + symmetricAlgorithmKey.get()); - String token = queryParams.get(JWT); + List authorizationHeaders = + requestEntity.getHeaders().get(HttpHeaders.AUTHORIZATION); + String token = + (authorizationHeaders != null && !authorizationHeaders.isEmpty()) + ? authorizationHeaders.get(0) + : null; if (token != null) { boolean isValid = jwtValidator.customHMACValidator( diff --git a/src/main/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjection.java b/src/main/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjection.java index e312fbfee..16b33b6aa 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjection.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjection.java @@ -70,6 +70,61 @@ private ResponseEntity getURLRedirectionResponseEntity( return new ResponseEntity<>(HttpStatus.OK); } + /** + * Browsers silently strip ASCII control characters (e.g. tab, newline) from a URL before + * parsing it, and normalize backslashes to forward slashes. A naive {@code startsWith} + * blacklist that only inspects the raw, un-normalized string can therefore be bypassed with + * payloads such as {@code "/\t/evil.com"} (control char hides a leading "//") or {@code + * "\/\/evil.com"} (backslashes stand in for the forward slashes). This helper normalizes the + * candidate the same way a browser would before applying the scheme/authority checks, so + * those bypasses collapse back into the already-blocked "//" / scheme-prefixed cases. + */ + private boolean isNotProtocolRelativeOrAbsolute(String url) { + String normalized = url.replace('\\', '/'); + for (int i = 0; i < normalized.length(); i++) { + if (normalized.charAt(i) <= ' ') { + // any embedded control/whitespace character is itself suspicious and is never + // part of a legitimate relative path we generate + return false; + } + } + return !normalized.startsWith(FrameworkConstants.HTTP) + && !normalized.startsWith(FrameworkConstants.HTTPS) + && !normalized.startsWith(FrameworkConstants.WWW) + && !normalized.startsWith("//") + && !normalized.startsWith(NULL_BYTE_CHARACTER); + } + + /** + * Builds a same-origin redirect path out of untrusted user input. Unlike naively concatenating + * {@code authority + urlToRedirect}, this guarantees exactly one leading slash and strips + * characters ("@", backslash, control chars) that could otherwise be abused to smuggle + * userinfo (e.g. {@code http://trusted.com@evil.com}) or a protocol-relative host into the + * resulting Location header. + */ + private String toSameOriginPath(String urlToRedirect) { + if (urlToRedirect == null) { + return "/"; + } + String normalized = urlToRedirect.replace('\\', '/'); + StringBuilder sanitized = new StringBuilder(); + for (int i = 0; i < normalized.length(); i++) { + char c = normalized.charAt(i); + if (c <= ' ' || c == '@') { + continue; + } + sanitized.append(c); + } + String result = sanitized.toString(); + while (result.startsWith("//")) { + result = result.substring(1); + } + if (!result.startsWith("/")) { + result = "/" + result; + } + return result; + } + @AttackVector( vulnerabilityExposed = {VulnerabilityType.OPEN_REDIRECT_3XX_STATUS_CODE}, description = "OPEN_REDIRECT_QUERY_PARAM_DIRECTLY_ADD_TO_LOCATION_HEADER") @@ -225,12 +280,7 @@ public ResponseEntity getVulnerablePayloadLevel5( return this.getURLRedirectionResponseEntity( urlToRedirect, (url) -> - (!url.startsWith(FrameworkConstants.HTTP) - && !url.startsWith(FrameworkConstants.HTTPS) - && !url.startsWith("//") - && !url.startsWith(FrameworkConstants.WWW) - && !url.startsWith(NULL_BYTE_CHARACTER) - && (url.length() > 0 && url.charAt(0) > 20)) + (url.length() > 0 && this.isNotProtocolRelativeOrAbsolute(url)) || requestUrl.getAuthority().equals(url)); } @@ -262,7 +312,11 @@ public ResponseEntity getVulnerablePayloadLevel6( headerParam.put(LOCATION_HEADER_KEY, new ArrayList<>()); headerParam .get(LOCATION_HEADER_KEY) - .add(requestUrl.getProtocol() + "://" + requestUrl.getAuthority() + urlToRedirect); + .add( + requestUrl.getProtocol() + + "://" + + requestUrl.getAuthority() + + this.toSameOriginPath(urlToRedirect)); return new ResponseEntity<>(headerParam, HttpStatus.FOUND); } @@ -290,17 +344,13 @@ public ResponseEntity getVulnerablePayloadLevel7( MultiValueMap headerParam = new org.springframework.http.HttpHeaders(); URL requestUrl = new URL(requestEntity.getUrl().toString()); headerParam.put(LOCATION_HEADER_KEY, new ArrayList<>()); - if (urlToRedirect.startsWith("/")) { - urlToRedirect = urlToRedirect.substring(1); - } headerParam .get(LOCATION_HEADER_KEY) .add( requestUrl.getProtocol() + "://" + requestUrl.getAuthority() - + "/" - + urlToRedirect); + + this.toSameOriginPath(urlToRedirect)); return new ResponseEntity<>(headerParam, HttpStatus.FOUND); } @@ -334,8 +384,14 @@ public ResponseEntity getVulnerablePayloadLevel8( value = LevelConstants.LEVEL_9, htmlTemplate = "LEVEL_9/Http3xxStatusCodeBasedInjection") public ResponseEntity getVulnerablePayloadLevel9( - @RequestParam(RETURN_TO) String urlToRedirect) { - return this.getURLRedirectionResponseEntity(urlToRedirect, (url) -> true); + RequestEntity requestEntity, @RequestParam(RETURN_TO) String urlToRedirect) + throws MalformedURLException { + URL requestUrl = new URL(requestEntity.getUrl().toString()); + return this.getURLRedirectionResponseEntity( + urlToRedirect, + (url) -> + (url.length() > 0 && this.isNotProtocolRelativeOrAbsolute(url)) + || requestUrl.getAuthority().equals(url)); } // Payloads: any URL e.g. /VulnerableApp/phishing/fake-login.html @@ -359,8 +415,14 @@ public ResponseEntity getVulnerablePayloadLevel9( value = LevelConstants.LEVEL_10, htmlTemplate = "LEVEL_10/Http3xxStatusCodeBasedInjection") public ResponseEntity getVulnerablePayloadLevel10( - @RequestParam(RETURN_TO) String urlToRedirect) { - return this.getURLRedirectionResponseEntity(urlToRedirect, (url) -> true); + RequestEntity requestEntity, @RequestParam(RETURN_TO) String urlToRedirect) + throws MalformedURLException { + URL requestUrl = new URL(requestEntity.getUrl().toString()); + return this.getURLRedirectionResponseEntity( + urlToRedirect, + (url) -> + (url.length() > 0 && this.isNotProtocolRelativeOrAbsolute(url)) + || requestUrl.getAuthority().equals(url)); } @AttackVector( diff --git a/src/main/resources/static/templates/JWTVulnerability/LEVEL_1/JWT_Level1.js b/src/main/resources/static/templates/JWTVulnerability/LEVEL_1/JWT_Level1.js index f6f172a91..2f87610f8 100644 --- a/src/main/resources/static/templates/JWTVulnerability/LEVEL_1/JWT_Level1.js +++ b/src/main/resources/static/templates/JWTVulnerability/LEVEL_1/JWT_Level1.js @@ -10,10 +10,12 @@ addingEventListenerToFetchTokenButton(); function addingEventListenerToVerifyToken() { document.getElementById("verifyToken").addEventListener("click", function () { let url = getUrlForVulnerabilityLevel(); - url = url + "?JWT=" + document.getElementById("jwt").value; - console.log(url); - console.log(document.getElementById("jwt").value); - doGetAjaxCall(updateUIWithVerifyResponse, url, true); + // The JWT is sent as an Authorization header rather than a URL query + // parameter so it is never written to browser history, server access + // logs, or a Referer header. + doGetAjaxCall(updateUIWithVerifyResponse, url, true, { + Authorization: document.getElementById("jwt").value, + }); }); } addingEventListenerToVerifyToken(); diff --git a/src/test/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerabilityTest.java b/src/test/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerabilityTest.java index 12b3b1cbd..eec0e792a 100644 --- a/src/test/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerabilityTest.java +++ b/src/test/java/org/sasanlabs/service/vulnerability/idor/IDORVulnerabilityTest.java @@ -34,29 +34,29 @@ void setup() { } @Test - void level1_ShouldAllowAccessToAnyId() { + void level1_ShouldAllowAccessToOwnId() { String validToken = "valid-token"; User decoded = new User(); decoded.setUserId(1); decoded.setRole("USER"); - User bob = new User(2, "Bob", 60000, "USER"); + User alice = new User(1, "Alice", 50000, "USER"); when(idorLoginService.decodeToken(validToken)).thenReturn(decoded); when(jdbcTemplate.query( anyString(), any(Object[].class), any(org.springframework.jdbc.core.RowMapper.class))) - .thenReturn(Arrays.asList(bob)); + .thenReturn(Arrays.asList(alice)); ResponseEntity> response = - idor.level1(validToken, 2); + idor.level1(validToken, 1); assertTrue(response.getBody().getIsValid()); } @Test - void level2_ShouldAllowCookieTampering() { - String validToken = "valid-token-level2"; + void level1_ShouldDenyAccessToAnotherUsersId() { + String validToken = "valid-token"; User decoded = new User(); decoded.setUserId(1); decoded.setRole("USER"); @@ -70,13 +70,35 @@ void level2_ShouldAllowCookieTampering() { .thenReturn(Arrays.asList(bob)); ResponseEntity> response = - idor.level2(validToken, 1); + idor.level1(validToken, 2); + + assertFalse(response.getBody().getIsValid()); + } + + @Test + void level2_ShouldIgnoreTamperedUserIdCookieAndUseTokenIdentity() { + String validToken = "valid-token-level2"; + User decoded = new User(); + decoded.setUserId(1); + decoded.setRole("USER"); + User alice = new User(1, "Alice", 50000, "USER"); + + when(idorLoginService.decodeToken(validToken)).thenReturn(decoded); + when(jdbcTemplate.query( + anyString(), eq(new Object[] {1}), any(org.springframework.jdbc.core.RowMapper.class))) + .thenReturn(Arrays.asList(alice)); + + // loggedInUser cookie is tampered to claim user id 2 (Bob), but the token itself belongs + // to user id 1 (Alice) - the tampered cookie must be ignored. + ResponseEntity> response = + idor.level2(validToken, 2); assertTrue(response.getBody().getIsValid()); + assertEquals("Alice", ((User) response.getBody().getContent()).getUsername()); } @Test - void level3_ShouldAllowRoleEscalationWhenRoleCookieIsAdmin() { + void level3_ShouldIgnoreRoleCookieTamperingAndDenyEscalation() { String fakeToken = java.util.Base64.getEncoder() .encodeToString("{\"userId\":2,\"role\":\"USER\"}".getBytes()); @@ -84,47 +106,40 @@ void level3_ShouldAllowRoleEscalationWhenRoleCookieIsAdmin() { User decoded = new User(); decoded.setUserId(2); decoded.setRole("USER"); - User bob = new User(3, "Charlie", 70000, "USER"); when(idorLoginService.decodeToken(fakeToken)).thenReturn(decoded); - when(jdbcTemplate.query( - anyString(), - any(Object[].class), - any(org.springframework.jdbc.core.RowMapper.class))) - .thenReturn(Arrays.asList(bob)); + // role_level3 cookie is tampered to claim ADMIN, but the signed token says USER and the + // requested id (3) does not belong to the token's own user (2) - access must be denied. ResponseEntity> response = idor.level3(fakeToken, "ADMIN", 3); - assertTrue(response.getBody().getIsValid()); + assertFalse(response.getBody().getIsValid()); } @Test - void level4_ShouldAllowOpaqueIdAccess() { + void level4_ShouldIgnoreEncodedRoleCookieTamperingAndDenyEscalation() { String encodedRole = java.util.Base64.getUrlEncoder() .withoutPadding() .encodeToString("ADMIN".getBytes()); - String escalatedToken = + String userToken = java.util.Base64.getEncoder() .encodeToString("{\"userId\":1,\"role\":\"USER\"}".getBytes()); User decoded = new User(); decoded.setUserId(1); decoded.setRole("USER"); - User bob = new User(2, "Bob", 60000, "USER"); - when(idorLoginService.decodeToken(escalatedToken)).thenReturn(decoded); - when(jdbcTemplate.query( - anyString(), - any(Object[].class), - any(org.springframework.jdbc.core.RowMapper.class))) - .thenReturn(Arrays.asList(bob)); + when(idorLoginService.decodeToken(userToken)).thenReturn(decoded); + // role_level4 cookie is base64-encoded "ADMIN", but encoding is not a signature - the + // signed token still says USER and the requested id (2) is not the token's own user (1), + // so access must be denied. ResponseEntity> response = - idor.level4(escalatedToken, encodedRole, 2); + idor.level4(userToken, encodedRole, 2); - assertTrue(response.getBody().getIsValid()); + assertFalse(response.getBody().getIsValid()); } @Test @@ -172,34 +187,6 @@ void level3_ShouldRejectInvalidToken() { assertEquals("Invalid token", response.getBody().getContent()); } - @Test - void level4_ShouldAllowAccessToAnyOpaqueIdRegardlessOfRole() { - String userToken = - java.util.Base64.getEncoder() - .encodeToString("{\"userId\":1,\"role\":\"USER\"}".getBytes()); - String encodedRole = - java.util.Base64.getUrlEncoder() - .withoutPadding() - .encodeToString("ADMIN".getBytes()); - - User decoded = new User(); - decoded.setUserId(1); - decoded.setRole("USER"); - User bob = new User(2, "Bob", 60000, "USER"); - - when(idorLoginService.decodeToken(userToken)).thenReturn(decoded); - when(jdbcTemplate.query( - anyString(), - any(Object[].class), - any(org.springframework.jdbc.core.RowMapper.class))) - .thenReturn(Arrays.asList(bob)); - - ResponseEntity> response = - idor.level4(userToken, encodedRole, 2); - - assertTrue(response.getBody().getIsValid()); - } - @Test void level5_ShouldAllowAdminFromDatabase() { String adminToken = diff --git a/src/test/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerabilityTest.java b/src/test/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerabilityTest.java index 1ecc8ef60..5dae33c42 100644 --- a/src/test/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerabilityTest.java +++ b/src/test/java/org/sasanlabs/service/vulnerability/jwt/JWTVulnerabilityTest.java @@ -168,11 +168,23 @@ private static void verifySymmetricAlgorithmKeyCreation(KeyStrength keyStrength) .getSymmetricAlgorithmKey(JWTUtils.JWT_HMAC_SHA_256_ALGORITHM, keyStrength); } + // Level 1 now reads the token to verify from the Authorization header instead of a "JWT" + // URL query parameter, so a bearer credential no longer ends up in the URL (browser + // history/server access logs/Referer headers). + private static RequestEntity getLevel1AuthorizationTokenRequest(String token) { + RequestEntity noAuthRequest = RequestEntity.get("/").build(); + if (token == null) { + return noAuthRequest; + } + return RequestEntity.get("/").header(HttpHeaders.AUTHORIZATION, token).build(); + } + @Test @DisplayName("Level 1 - Test that a token is generated if none is submitted") void testLevel1Creation() throws Exception { ResponseEntity> response = - jwtVulnerability.getVulnerablePayloadLevelUnsecure(new HashMap<>()); + jwtVulnerability.getVulnerablePayloadLevelUnsecure( + getLevel1AuthorizationTokenRequest(null)); assertValidOkResponse(response); assertNotNull(response.getBody(), "Response body should not be null"); assertTokenInBody(response); @@ -182,10 +194,9 @@ void testLevel1Creation() throws Exception { @Test @DisplayName("Level 1 - Test that a valid token is validated successfully") void testLevel1SuccessfulValidation() throws Exception { - HashMap query = new HashMap<>(); - query.put(JWTVulnerability.JWT, validHighStrengthToken); ResponseEntity> response = - jwtVulnerability.getVulnerablePayloadLevelUnsecure(query); + jwtVulnerability.getVulnerablePayloadLevelUnsecure( + getLevel1AuthorizationTokenRequest(validHighStrengthToken)); assertValidOkResponse(response); assertNoTokenInBody(response); } @@ -193,10 +204,9 @@ void testLevel1SuccessfulValidation() throws Exception { @Test @DisplayName("Level 1 - Test that an invalid token is not validated successfully") void testLevel1FailedValidation() throws Exception { - HashMap query = new HashMap<>(); - query.put(JWTVulnerability.JWT, invalidToken); ResponseEntity> response = - jwtVulnerability.getVulnerablePayloadLevelUnsecure(query); + jwtVulnerability.getVulnerablePayloadLevelUnsecure( + getLevel1AuthorizationTokenRequest(invalidToken)); assertValidUnauthorizedResponse(response); assertEquals( response.getBody().getContent(), diff --git a/src/test/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjectionTest.java b/src/test/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjectionTest.java index 734c9c30b..f8aa9630f 100644 --- a/src/test/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjectionTest.java +++ b/src/test/java/org/sasanlabs/service/vulnerability/openRedirect/Http3xxStatusCodeBasedInjectionTest.java @@ -521,10 +521,16 @@ void test_That_ReturnToQueryParameterValue_IsNotAddedToLocationHeader_WhenItStar @Test @DisplayName( - "Level 6- test that the returnTo query parameter's value is directly added to the Location header by adding domain as prefix") + "Level 6- test that the returnTo query parameter's value is safely appended as a same-origin path instead of being fused onto the trusted domain") void test_That_ReturnToQueryParameterValue_IsAddedToLocationHeader_ByAddingDomainToPrefix_Level6() throws MalformedURLException, URISyntaxException { + // Naively concatenating authority + "somedomain.com" (no separating slash) used to + // produce "https://somedomain.comsomedomain.com" - and with an attacker-chosen value + // like ".evil.com" this pattern is the classic "domain as prefix" bypass, since + // "https://somedomain.com.evil.com" is a subdomain of evil.com, not of somedomain.com. + // The fix guarantees exactly one separating slash so the result always stays a path + // under the trusted origin. String redirectUrl = "somedomain.com"; RequestEntity requestEntity = new RequestEntity<>( @@ -534,7 +540,26 @@ void test_That_ReturnToQueryParameterValue_IsNotAddedToLocationHeader_WhenItStar requestEntity, redirectUrl); assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.FOUND); assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)) - .contains("https://somedomain.comsomedomain.com"); + .contains("https://somedomain.com/somedomain.com"); + } + + @Test + @DisplayName( + "Level 6- test that a domain-suffix-confusion payload does not escape the trusted origin") + void test_That_DomainSuffixConfusionPayload_DoesNotEscapeTrustedOrigin_Level6() + throws MalformedURLException, URISyntaxException { + String redirectUrl = ".evil.com"; + RequestEntity requestEntity = + new RequestEntity<>( + HttpMethod.GET, new URI("https://somedomain.com?returnTo=.evil.com")); + ResponseEntity responseEntity = + http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel6( + requestEntity, redirectUrl); + assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.FOUND); + assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)) + .contains("https://somedomain.com/.evil.com"); + assertThat(responseEntity.getHeaders().getFirst(LOCATION_HEADER_KEY)) + .doesNotContain("somedomain.com.evil.com"); } @Test @@ -591,35 +616,69 @@ void test_That_ReturnToQueryParameterValue_IsNotAddedToLocationHeader_WhenItStar } @Test - @DisplayName("Level 9 - test that URL provided in returnTo parameter results in a 302 redirect") - void test_that_ReturnToQueryParameterValue_IsAddedToLocationHeader_Level9() { + @DisplayName( + "Level 9 - test that a same-site relative URL provided in returnTo parameter results in a 302 redirect") + void test_that_ReturnToQueryParameterValue_IsAddedToLocationHeader_Level9() + throws URISyntaxException, MalformedURLException { String phishingURL = "/VulnerableApp/phishing/fake-login.html"; + RequestEntity requestEntity = + new RequestEntity<>( + HttpMethod.GET, + new URI("https://somedomain.com?returnTo=" + phishingURL)); ResponseEntity responseEntity = - http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel9(phishingURL); + http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel9( + requestEntity, phishingURL); assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.FOUND); assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)).contains(phishingURL); } @Test @DisplayName( - "Level 10- test that URL provided in returnTo parameter results in a 302 redirect with Location header set") - void test_That_ReturnToQueryParameterValue_IsAddedToLocationHeader_Level10() { + "Level 9 - test that an external malicious URL provided in returnTo parameter is rejected") + void test_That_ExternalMaliciousUrl_IsRejected_Level9() throws URISyntaxException, MalformedURLException { + String redirectUrl = "https://www.malicious.com"; + RequestEntity requestEntity = + new RequestEntity<>( + HttpMethod.GET, + new URI("https://somedomain.com?returnTo=" + redirectUrl)); + ResponseEntity responseEntity = + http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel9( + requestEntity, redirectUrl); + assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)).isEqualTo(null); + } + + @Test + @DisplayName( + "Level 10- test that a same-site relative URL provided in returnTo parameter results in a 302 redirect with Location header set") + void test_That_ReturnToQueryParameterValue_IsAddedToLocationHeader_Level10() + throws URISyntaxException, MalformedURLException { String redirectUrl = "/VulnerableApp/phishing/fake-login.html"; + RequestEntity requestEntity = + new RequestEntity<>( + HttpMethod.GET, + new URI("https://somedomain.com?returnTo=" + redirectUrl)); ResponseEntity responseEntity = - http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel10(redirectUrl); + http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel10( + requestEntity, redirectUrl); assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.FOUND); assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)).contains(redirectUrl); } @Test @DisplayName( - "Level 10- test that an external malicious URL is accepted and results in a 302 redirect without any domain restriction") - void test_That_ExternalMaliciousUrl_IsAccepted_AndAddedToLocationHeader_Level10() { + "Level 10- test that an external malicious URL is rejected and does not result in a redirect") + void test_That_ExternalMaliciousUrl_IsRejected_Level10() throws URISyntaxException, MalformedURLException { String redirectUrl = "https://www.malicious.com"; + RequestEntity requestEntity = + new RequestEntity<>( + HttpMethod.GET, + new URI("https://somedomain.com?returnTo=" + redirectUrl)); ResponseEntity responseEntity = - http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel10(redirectUrl); - assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.FOUND); - assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)).contains(redirectUrl); + http3xxStatusCodeBasedInjection.getVulnerablePayloadLevel10( + requestEntity, redirectUrl); + assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(responseEntity.getHeaders().get(LOCATION_HEADER_KEY)).isEqualTo(null); } @Test