From 6acfb38132b75d959a18da8eaad4bdd0eeb96df3 Mon Sep 17 00:00:00 2001
From: Mohamed Karray <mohamed.karray.sr@gmail.com>
Date: Thu, 6 Sep 2018 13:54:52 +0200
Subject: [PATCH] add crlf exception and exception mapper

---
 .../sonia/scm/util/CRLFInjectionException.java   |  8 ++++++++
 .../src/main/java/sonia/scm/util/HttpUtil.java   |  3 +--
 .../resources/CRLFInjectionExceptionMapper.java  | 13 +++++++++++++
 .../scm/api/v2/resources/DiffRootResource.java   | 12 ++++++++++--
 .../scm/api/v2/resources/DiffResourceTest.java   | 16 ++++++++++++++++
 5 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java
 create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java

diff --git a/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java b/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java
new file mode 100644
index 0000000000..16dca8e910
--- /dev/null
+++ b/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java
@@ -0,0 +1,8 @@
+package sonia.scm.util;
+
+public class CRLFInjectionException extends IllegalArgumentException{
+
+  public CRLFInjectionException(String message) {
+    super(message);
+  }
+}
diff --git a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java
index bc3f4e74cd..2744addc62 100644
--- a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java
+++ b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java
@@ -344,8 +344,7 @@ public final class HttpUtil
         "parameter \"{}\" contains a character which could be an indicator for a crlf injection",
         parameter);
 
-      throw new IllegalArgumentException(
-        "parameter contains an illegal character");
+      throw new CRLFInjectionException("parameter contains an illegal character");
     }
   }
 
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java
new file mode 100644
index 0000000000..f4e8d3aa3c
--- /dev/null
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java
@@ -0,0 +1,13 @@
+package sonia.scm.api.v2.resources;
+
+import sonia.scm.api.rest.StatusExceptionMapper;
+import sonia.scm.util.CRLFInjectionException;
+
+import javax.ws.rs.core.Response;
+
+public class CRLFInjectionExceptionMapper extends StatusExceptionMapper<CRLFInjectionException> {
+
+  public CRLFInjectionExceptionMapper() {
+    super(CRLFInjectionException.class, Response.Status.BAD_REQUEST);
+  }
+}
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java
index e98b3bc868..1a2b3d2189 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java
@@ -31,6 +31,16 @@ public class DiffRootResource {
     this.serviceFactory = serviceFactory;
   }
 
+
+  /**
+   * Get the repository diff of a revision
+   *
+   * @param namespace repository namespace
+   * @param name repository name
+   * @param revision the revision
+   * @return the dif of the revision
+   * @throws NotFoundException if the repository is not found
+   */
   @GET
   @Path("{revision}")
   @Produces(VndMediaType.DIFF)
@@ -45,8 +55,6 @@ public class DiffRootResource {
   public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision) throws NotFoundException {
     HttpUtil.checkForCRLFInjection(revision);
     try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) {
-      Repository repository = repositoryService.getRepository();
-      RepositoryPermissions.read(repository).check();
       StreamingOutput responseEntry = output -> {
         try {
           repositoryService.getDiffCommand()
diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java
index a6f9c26779..0daeb07b7c 100644
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java
@@ -72,6 +72,7 @@ public class DiffResourceTest {
     when(service.getRepository()).thenReturn(new Repository("repoId", "git", "space", "repo"));
     dispatcher.getProviderFactory().registerProvider(NotFoundExceptionMapper.class);
     dispatcher.getProviderFactory().registerProvider(AuthorizationExceptionMapper.class);
+    dispatcher.getProviderFactory().registerProvider(CRLFInjectionExceptionMapper.class);
     when(service.getDiffCommand()).thenReturn(diffCommandBuilder);
     subjectThreadState.bind();
     ThreadContext.bind(subject);
@@ -130,4 +131,19 @@ public class DiffResourceTest {
     assertEquals(404, response.getStatus());
   }
 
+  @Test
+  public void shouldGet400OnCrlfInjection() throws Exception {
+    when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder);
+    when(diffCommandBuilder.retriveContent(any())).thenThrow(RevisionNotFoundException.class);
+
+    MockHttpRequest request = MockHttpRequest
+      .get(DIFF_URL + "ny%0D%0ASet-cookie:%20Tamper=3079675143472450634")
+      .accept(VndMediaType.DIFF);
+    MockHttpResponse response = new MockHttpResponse();
+    dispatcher.invoke(request, response);
+    assertEquals(400, response.getStatus());
+  }
+
+
+
 }