From 49b1a9dddc098ae0a07a321a376123496ae329fd Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 19 Jun 2026 09:20:54 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20CRLF=20injection=20in=20Content-Disposition=20header?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit sanitizes the `filename` parameter in the `Content-Disposition` header in `DownloadGetRequestHandler.java`. By removing carriage returns (`\r`), line feeds (`\n`), and double quotes (`"`), we prevent HTTP Response Splitting / CRLF Injection vulnerabilities. The URL-encoded `filename*` parameter remains fully capable of handling special characters safely. Also added `DownloadGetRequestHandlerSecurityTest.java` to prevent regression. Co-authored-by: tomdesair <14034630+tomdesair@users.noreply.github.com> --- .jules/sentinel.md | 4 ++ .../download/DownloadGetRequestHandler.java | 2 +- ...DownloadGetRequestHandlerSecurityTest.java | 64 +++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 .jules/sentinel.md create mode 100644 src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java 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..8293513 --- /dev/null +++ b/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java @@ -0,0 +1,64 @@ +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.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +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")); + } +} From 6a856b2d5fbdd345a36166c39f67a598c0ac9caa Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 19 Jun 2026 09:24:43 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20CRLF=20injection=20in=20Content-Disposition=20header?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit sanitizes the `filename` parameter in the `Content-Disposition` header in `DownloadGetRequestHandler.java`. By removing carriage returns (`\r`), line feeds (`\n`), and double quotes (`"`), we prevent HTTP Response Splitting / CRLF Injection vulnerabilities. The URL-encoded `filename*` parameter remains fully capable of handling special characters safely. Also added `DownloadGetRequestHandlerSecurityTest.java` to prevent regression. Co-authored-by: tomdesair <14034630+tomdesair@users.noreply.github.com> --- .../DownloadGetRequestHandlerSecurityTest.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java b/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java index 8293513..76ac076 100644 --- a/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java +++ b/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerSecurityTest.java @@ -4,9 +4,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Base64; @@ -49,7 +47,8 @@ public void testCrlfInjectionInFileName() throws Exception { 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()); + String encodedMeta = + "filename " + Base64.getEncoder().encodeToString(maliciousFileName.getBytes()); info.setEncodedMetadata(encodedMeta); when(uploadStorageService.getUploadInfo(anyString(), any())).thenReturn(info); @@ -59,6 +58,10 @@ public void testCrlfInjectionInFileName() throws Exception { 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")); + assertThat( + contentDisposition, + is( + "attachment; filename=\"testInjected: true.txt\";" + + " filename*=UTF-8''test%0D%0AInjected%3A%20true.txt%22")); } }