diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..f0607ca --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2026-06-19 - [Fix CRLF injection in Content-Disposition header] +**Vulnerability:** A CRLF injection vulnerability was present in `DownloadGetRequestHandler.java` where the unsanitized `info.getFileName()` was passed to `String.format` to form the `Content-Disposition` header. An attacker could craft a malicious filename containing carriage returns, line feeds, and double quotes to inject arbitrary HTTP headers in the response. +**Learning:** Legacy HTTP header fields that accept unencoded strings, like the `filename="..."` attribute in `Content-Disposition`, are a common vector for HTTP response splitting/CRLF injection. +**Prevention:** Always sanitize untrusted input when constructing HTTP headers, especially when not using a built-in framework that automatically sanitizes them. For filenames in `Content-Disposition`, strip out `\r`, `\n`, and `"` from the unencoded parameter, and ensure the `filename*=` parameter uses proper URL encoding (e.g., `URLEncoder.encode`). diff --git a/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java b/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java index 4a9aa23..9980abb 100644 --- a/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java +++ b/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java @@ -50,7 +50,7 @@ public void process( HttpHeader.CONTENT_DISPOSITION, String.format( CONTENT_DISPOSITION_FORMAT, - info.getFileName(), + info.getFileName().replaceAll("[\r\n\"]", ""), URLEncoder.encode(info.getFileName(), StandardCharsets.UTF_8.toString()) .replace("+", "%20"))); diff --git a/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java b/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java new file mode 100644 index 0000000..76ac076 --- /dev/null +++ b/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java @@ -0,0 +1,67 @@ +package me.desair.tus.server.download; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Base64; +import me.desair.tus.server.HttpHeader; +import me.desair.tus.server.HttpMethod; +import me.desair.tus.server.upload.UploadId; +import me.desair.tus.server.upload.UploadInfo; +import me.desair.tus.server.upload.UploadStorageService; +import me.desair.tus.server.util.TusServletRequest; +import me.desair.tus.server.util.TusServletResponse; +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +public class DownloadGetRequestHandlerSecurityTest { + + private DownloadGetRequestHandler handler; + private MockHttpServletRequest servletRequest; + private MockHttpServletResponse servletResponse; + private TusServletRequest tusRequest; + private TusServletResponse tusResponse; + private UploadStorageService uploadStorageService; + + @Before + public void setUp() { + handler = new DownloadGetRequestHandler(); + servletRequest = new MockHttpServletRequest(); + servletResponse = new MockHttpServletResponse(); + tusRequest = new TusServletRequest(servletRequest); + tusResponse = new TusServletResponse(servletResponse); + uploadStorageService = mock(UploadStorageService.class); + } + + @Test + public void testCrlfInjectionInFileName() throws Exception { + UploadInfo info = new UploadInfo(); + info.setId(new UploadId("123")); + info.setLength(10L); + info.setOffset(10L); // finished + // filename: "test\r\nInjected: true.txt" + String maliciousFileName = "test\r\nInjected: true.txt\""; + String encodedMeta = + "filename " + Base64.getEncoder().encodeToString(maliciousFileName.getBytes()); + info.setEncodedMetadata(encodedMeta); + + when(uploadStorageService.getUploadInfo(anyString(), any())).thenReturn(info); + + handler.process(HttpMethod.GET, tusRequest, tusResponse, uploadStorageService, null); + + String contentDisposition = servletResponse.getHeader(HttpHeader.CONTENT_DISPOSITION); + + // The filename parameter should not contain \r, \n, or " + assertThat( + contentDisposition, + is( + "attachment; filename=\"testInjected: true.txt\";" + + " filename*=UTF-8''test%0D%0AInjected%3A%20true.txt%22")); + } +}