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 adc268acd5..ad1ff64760 100644 --- a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java +++ b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java @@ -243,7 +243,6 @@ public class HttpUtil * * @param parameter value * - * @return true if the request comes from the web interface. * @since 1.28 */ public static void checkForCRLFInjection(String parameter) @@ -350,6 +349,22 @@ public class HttpUtil return url; } + /** + * Remove all chars from the given parameter, which could be used for + * CRLF injection attack. Note: the current implementation + * the "%" char is also removed from the source parameter. + * + * @param parameter value + * + * @return the parameter value without crlf chars + * + * @since 1.28 + */ + public static String removeCRLFInjectionChars(String parameter) + { + return CRLF_CHARMATCHER.removeFrom(parameter); + } + /** * Method description * diff --git a/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java b/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java index 750f5a7d70..d2483c219f 100644 --- a/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java +++ b/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java @@ -141,6 +141,22 @@ public class HttpUtilTest HttpUtil.checkForCRLFInjection("abcka"); } + /** + * Method description + * + */ + @Test + public void testRemoveCRLFInjectionChars() + { + assertEquals("any0D0A", HttpUtil.removeCRLFInjectionChars("any%0D%0A")); + assertEquals("123abc", HttpUtil.removeCRLFInjectionChars("123\nabc")); + assertEquals("123abc", HttpUtil.removeCRLFInjectionChars("123\r\nabc")); + assertEquals("123abc", HttpUtil.removeCRLFInjectionChars("123%abc")); + assertEquals("123abc", HttpUtil.removeCRLFInjectionChars("123abc")); + assertEquals("123", HttpUtil.removeCRLFInjectionChars("123")); + + } + //~--- get methods ---------------------------------------------------------- /** diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java index 75c0a7b7d0..738fd45822 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java @@ -793,6 +793,12 @@ public class RepositoryResource output = new BrowserStreamingOutput(service, builder, path); + /** + * protection for crlf injection + * see https://bitbucket.org/sdorra/scm-manager/issue/320/crlf-injection-vulnerability-in-diff-api + */ + path = HttpUtil.removeCRLFInjectionChars(path); + String contentDispositionName = getContentDispositionNameFromPath(path); response = Response.ok(output).header("Content-Disposition", @@ -849,7 +855,7 @@ public class RepositoryResource AssertUtil.assertIsNotEmpty(revision); /** - * HttpUtil.checkForCRLFInjection(revision); + * check for a crlf injection attack * see https://bitbucket.org/sdorra/scm-manager/issue/320/crlf-injection-vulnerability-in-diff-api */ HttpUtil.checkForCRLFInjection(revision);