SONAR-6881 Disable OPTIONS, HEAD and TRACE methods of web server

diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/SecurityServletFilter.java b/server/sonar-server/src/main/java/org/sonar/server/platform/SecurityServletFilter.java
index f4d3b486a27e9591037fc146e2004a38ed4c9319..702a6baa2b253f59a0f366729eb387221ba6b623 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/platform/SecurityServletFilter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/platform/SecurityServletFilter.java
@@ -19,22 +19,26 @@
  */
 package org.sonar.server.platform;
 
+import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
+import java.util.Set;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import java.io.IOException;
-
 /**
  * This servlet filter sets response headers that enable browser protection against several classes if Web attacks.
  * The list of headers is mirrored in environment.rb as a workaround to Rack swallowing the headers..
  */
 public class SecurityServletFilter implements Filter {
 
+  private static final Set<String> ALLOWED_HTTP_METHODS = ImmutableSet.of("DELETE", "GET", "POST", "PUT");
+
   @Override
   public void init(FilterConfig filterConfig) throws ServletException {
     // nothing
@@ -42,11 +46,20 @@ public class SecurityServletFilter implements Filter {
 
   @Override
   public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException, ServletException {
-    chain.doFilter(req, resp);
+    doHttpFilter((HttpServletRequest) req, (HttpServletResponse) resp, chain);
+  }
+
+  private static void doHttpFilter(HttpServletRequest httpRequest, HttpServletResponse httpResponse, FilterChain chain) throws IOException, ServletException {
+    // SONAR-6881 Disable OPTIONS, HEAD and TRACE methods
+    if (!ALLOWED_HTTP_METHODS.contains(httpRequest.getMethod())) {
+      httpResponse.setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+      return;
+    }
+
+    chain.doFilter(httpRequest, httpResponse);
 
     // Clickjacking protection
     // See https://www.owasp.org/index.php/Clickjacking_Protection_for_Java_EE
-    HttpServletResponse httpResponse = (HttpServletResponse) resp;
     httpResponse.addHeader("X-Frame-Options", "SAMEORIGIN");
 
     // Cross-site scripting

diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/SecurityServletFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/SecurityServletFilterTest.java
index 79edf7605667038c728766d17787c833afcfd615..0f3adb127b162e94e4dc9745285e85b6acc13f85 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/platform/SecurityServletFilterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/platform/SecurityServletFilterTest.java
@@ -19,33 +19,69 @@
  */
 package org.sonar.server.platform;
 
-import org.junit.Test;
-
+import java.io.IOException;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import org.junit.Test;
 
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.startsWith;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class SecurityServletFilterTest {
 
+  SecurityServletFilter underTest = new SecurityServletFilter();
+  HttpServletResponse response = mock(HttpServletResponse.class);
+  FilterChain chain = mock(FilterChain.class);
+
+  @Test
+  public void accept_GET_method() throws IOException, ServletException {
+    HttpServletRequest request = newRequest("GET");
+    underTest.doFilter(request, response, chain);
+    verify(response, never()).setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+    verify(chain).doFilter(request, response);
+  }
+
+  @Test
+  public void deny_HEAD_method() throws IOException, ServletException {
+    underTest.doFilter(newRequest("HEAD"), response, chain);
+    verify(response).setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+  }
+
   @Test
-  public void set_secured_headers() throws Exception {
-    SecurityServletFilter filter = new SecurityServletFilter();
-    filter.init(mock(FilterConfig.class));
+  public void deny_OPTIONS_method() throws IOException, ServletException {
+    underTest.doFilter(newRequest("OPTIONS"), response, chain);
+    verify(response).setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+  }
 
-    HttpServletRequest request = mock(HttpServletRequest.class);
-    HttpServletResponse response = mock(HttpServletResponse.class);
-    FilterChain chain = mock(FilterChain.class);
-    filter.doFilter(request, response, chain);
+  @Test
+  public void deny_TRACE_method() throws IOException, ServletException {
+    underTest.doFilter(newRequest("TRACE"), response, chain);
+    verify(response).setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+  }
+
+  @Test
+  public void set_secured_headers() throws ServletException, IOException {
+    underTest.init(mock(FilterConfig.class));
+    HttpServletRequest request = newRequest("GET");
+
+    underTest.doFilter(request, response, chain);
 
     verify(response, times(3)).addHeader(startsWith("X-"), anyString());
 
-    filter.destroy();
+    underTest.destroy();
+  }
+
+  private HttpServletRequest newRequest(String httpMethod) {
+    HttpServletRequest req = mock(HttpServletRequest.class);
+    when(req.getMethod()).thenReturn(httpMethod);
+    return req;
   }
 }