SONAR-21889 Fix SSF-539

(cherry picked from commit b5abb73908a7963c9055351fed7d39398b335450)

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 633591f1023855d63fcd51e4b0cfbb10bc761694..9cdaa9d466c86514edbc43d80a7a441e53d8f59f 100644 (file)
--- 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
@@ -25,6 +25,7 @@ import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Type;
+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 252b6fcdf5e7d9d6c17850362168a7dd094d798d..aa499933316b02e96d29d86d9a0112af542aecfe 100644 (file)
--- 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 javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -54,7 +56,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void init_create_cookie() {
-    when(request.getParameter("return_to")).thenReturn("/settings");
+    when(request.getParameter("return_to")).thenReturn("/admin/settings");
 
     underTest.init(request, response);
 
@@ -114,7 +116,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);
 
@@ -123,7 +125,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void get_return_to_is_empty_when_no_value() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{}")});
 
     Optional<String> redirection = underTest.getReturnTo(request);
 
@@ -132,7 +134,7 @@ public class OAuth2AuthenticationParametersImplTest {
 
   @Test
   public void delete() {
-    when(request.getCookies()).thenReturn(new Cookie[] {new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/settings\"}")});
+    when(request.getCookies()).thenReturn(new Cookie[]{new Cookie(AUTHENTICATION_COOKIE_NAME, "{\"return_to\":\"/settings\"}")});
 
     underTest.delete(request, response);
 
@@ -143,4 +145,35 @@ public class OAuth2AuthenticationParametersImplTest {
     assertThat(updatedCookie.getPath()).isEqualTo("/");
     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 Cookie wrapCookie(String name, String value) {
+    return new javax.servlet.http.Cookie(name, value);
+  }
 }