From 9879018139f85505b5914f9d79915e8f2da5134e Mon Sep 17 00:00:00 2001
From: Julien Lancelot <julien.lancelot@sonarsource.com>
Date: Wed, 19 Feb 2020 10:11:56 +0100
Subject: [PATCH] SONAR-12862 Fix SSF-101

---
 .../OAuth2AuthenticationParametersImpl.java   | 14 ++++++++------
 ...Auth2AuthenticationParametersImplTest.java | 19 +++++++------------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java
index 848dfab0ee0..ef008e898dd 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImpl.java
@@ -28,6 +28,7 @@ import java.lang.reflect.Type;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
+import java.util.regex.Pattern;
 import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -44,6 +45,7 @@ public class OAuth2AuthenticationParametersImpl implements OAuth2AuthenticationP
 
   private static final String AUTHENTICATION_COOKIE_NAME = "AUTH-PARAMS";
   private static final int FIVE_MINUTES_IN_SECONDS = 5 * 60;
+  private static final Pattern VALID_RETURN_TO = Pattern.compile("^/\\w.*");
 
   /**
    * The HTTP parameter that contains the path where the user should be redirect to.
@@ -151,13 +153,13 @@ public class OAuth2AuthenticationParametersImpl implements OAuth2AuthenticationP
     if (Strings.isNullOrEmpty(url)) {
       return empty();
     }
-    if (url.startsWith("//") || url.startsWith("/\\")) {
-      return empty();
-    }
-    if (!url.startsWith("/")) {
+
+    String sanitizedUrl = url.trim();
+    boolean isValidUrl = VALID_RETURN_TO.matcher(sanitizedUrl).matches();
+    if (!isValidUrl) {
       return empty();
     }
-    return Optional.of(url);
-  }
 
+    return Optional.of(sanitizedUrl);
+  }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java
index f60db60416e..623270fe053 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/OAuth2AuthenticationParametersImplTest.java
@@ -19,12 +19,15 @@
  */
 package org.sonar.server.authentication;
 
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import java.util.Optional;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -34,6 +37,7 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+@RunWith(DataProviderRunner.class)
 public class OAuth2AuthenticationParametersImplTest {
 
   private static final String AUTHENTICATION_COOKIE_NAME = "AUTH-PARAMS";
@@ -95,21 +99,12 @@ public class OAuth2AuthenticationParametersImplTest {
   }
 
   @Test
-  public void return_to_is_not_set_when_not_local() {
-    when(request.getParameter("return_to")).thenReturn("http://external_url");
-    underTest.init(request, response);
-    verify(response, never()).addCookie(any());
+  @DataProvider({"http://example.com", "/\t/example.com", "//local_file", "/\\local_file", "something_else"})
+  public void return_to_is_not_set_when_not_local(String url) {
+    when(request.getParameter("return_to")).thenReturn(url);
 
-    when(request.getParameter("return_to")).thenReturn("//local_file");
     underTest.init(request, response);
-    verify(response, never()).addCookie(any());
 
-    when(request.getParameter("return_to")).thenReturn("/\\local_file");
-    underTest.init(request, response);
-    verify(response, never()).addCookie(any());
-
-    when(request.getParameter("return_to")).thenReturn("something_else");
-    underTest.init(request, response);
     verify(response, never()).addCookie(any());
   }
 
-- 
2.39.5