fix another possible crlf injection, see issue #320

This commit is contained in:
Sebastian Sdorra
2013-01-28 13:20:22 +01:00
parent 500a082a3f
commit e8e288ccf0
3 changed files with 39 additions and 2 deletions

View File

@@ -243,7 +243,6 @@ public class HttpUtil
* *
* @param parameter value * @param parameter value
* *
* @return true if the request comes from the web interface.
* @since 1.28 * @since 1.28
*/ */
public static void checkForCRLFInjection(String parameter) public static void checkForCRLFInjection(String parameter)
@@ -350,6 +349,22 @@ public class HttpUtil
return url; return url;
} }
/**
* Remove all chars from the given parameter, which could be used for
* CRLF injection attack. <stronng>Note:</strong> 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 * Method description
* *

View File

@@ -141,6 +141,22 @@ public class HttpUtilTest
HttpUtil.checkForCRLFInjection("abcka"); 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 ---------------------------------------------------------- //~--- get methods ----------------------------------------------------------
/** /**

View File

@@ -793,6 +793,12 @@ public class RepositoryResource
output = new BrowserStreamingOutput(service, builder, path); 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); String contentDispositionName = getContentDispositionNameFromPath(path);
response = Response.ok(output).header("Content-Disposition", response = Response.ok(output).header("Content-Disposition",
@@ -849,7 +855,7 @@ public class RepositoryResource
AssertUtil.assertIsNotEmpty(revision); 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 * see https://bitbucket.org/sdorra/scm-manager/issue/320/crlf-injection-vulnerability-in-diff-api
*/ */
HttpUtil.checkForCRLFInjection(revision); HttpUtil.checkForCRLFInjection(revision);