SONAR-21889 Fix SSF-539

2 files changed, 42 insertions, 7 deletions

diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java
index d65e483a221..7be6aed2680 100644
--- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java
+++ b/server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java
@@ -24,6 +24,7 @@ import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.reflect.TypeToken;
 import java.io.UnsupportedEncodingException;
+import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -124,12 +125,17 @@ public class OAuth2AuthenticationParametersImpl implements OAuth2AuthenticationP
       return empty();
     }
 
-    String sanitizedUrl = url.trim();
-    boolean isValidUrl = VALID_RETURN_TO.matcher(sanitizedUrl).matches();
+    String trimmedUrl = url.trim();
+    boolean isValidUrl = VALID_RETURN_TO.matcher(trimmedUrl).matches();
     if (!isValidUrl) {
       return empty();
     }
 
-    return Optional.of(sanitizedUrl);
+    Path sanitizedPath = escapePathTraversalChars(trimmedUrl);
+    return Optional.of(sanitizedPath.toString());
+  }
+
+  private static Path escapePathTraversalChars(String sanitizedUrl) {
+    return Path.of(sanitizedUrl).normalize();
   }
 }
diff --git a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java
index a66ed68df57..4c9b4d4b2fb 100644
--- a/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java
+++ b/server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java
@@ -21,7 +21,9 @@ package org.sonar.server.authentication;
 
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -106,7 +108,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_return_to_parameter() {
-    when(request.getCookies()).thenReturn(new Cookie[] {wrapCookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/admin/settings\"}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{wrapCookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/admin/settings\"}")});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
@@ -115,7 +117,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_return_to_is_empty_when_no_cookie() {
-    when(request.getCookies()).thenReturn(new Cookie[] {});
+    when(request.getCookies()).thenReturn(new Cookie[]{});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
@@ -124,7 +126,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_return_to_is_empty_when_no_value() {
-    when(request.getCookies()).thenReturn(new Cookie[] {wrapCookie(AUTHENTICATION_COOKIE_NAME, "{}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{wrapCookie(AUTHENTICATION_COOKIE_NAME, "{}")});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
@@ -133,7 +135,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void delete() {
-    when(request.getCookies()).thenReturn(new Cookie[] {wrapCookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/admin/settings\"}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{wrapCookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/admin/settings\"}")});
 
     underTest.delete(request, response);
 
@@ -145,6 +147,33 @@ public class OAuth2AuthenticationParametersImplTest {
     assertThat(updatedCookie.getMaxAge()).isZero();
   }
 
+  @DataProvider
+  public static Object[][] payloadToSanitizeAndExpectedOutcome() {
+    return new Object[][]{
+      {generatePath("/admin/settings"), "/admin/settings"},
+      {generatePath("/admin/../../settings"), "/settings"},
+      {generatePath("/admin/../settings"), "/settings"},
+      {generatePath("/admin/settings/.."), "/admin"},
+      {generatePath("/admin/..%2fsettings/"), "/settings"},
+      {generatePath("/admin/%2e%2e%2fsettings/"), "/settings"},
+      {generatePath("../admin/settings"), null},
+    };
+  }
+
+  private static String generatePath(String returnTo) {
+    return "{\"return_to\":\"" + returnTo + "\"}";
+  }
+
+  @Test
+  @UseDataProvider("payloadToSanitizeAndExpectedOutcome")
+  public void getReturnTo_whenContainingPathTraversalCharacters_sanitizeThem(String payload, @Nullable String expectedSanitizedUrl) {
+    when(request.getCookies()).thenReturn(new Cookie[]{wrapCookie(AUTHENTICATION_COOKIE_NAME, payload)});
+
+    Optional<String> redirection = underTest.getReturnTo(request);
+
+    assertThat(redirection).isEqualTo(Optional.ofNullable(expectedSanitizedUrl));
+  }
+
   private JavaxHttpRequest.JavaxCookie wrapCookie(String name, String value) {
     return new JavaxHttpRequest.JavaxCookie(new javax.servlet.http.Cookie(name, value));
   }