From fbddc6af26c8fc5d7c4ddc0354aeb1316377bbb2 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 09:18:56 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20global=20CRLF=20injection=20in=20HTTP=20headers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces a robust, global defense against HTTP Response Splitting and CRLF Injection vulnerabilities. Instead of patching individual endpoints and headers (such as Content-Disposition or Upload-Metadata) as they arise, `TusServletResponse.java` has been modified to intrinsically sanitize all outgoing header values by stripping carriage return (`\r`) and line feed (`\n`) characters before they are passed to the underlying HttpServletResponse wrapper. The specialized `DownloadGetRequestHandlerCRLFTest.java` was removed and replaced with a broader `TusServletResponseCRLFTest.java` that covers `setHeader` and `addHeader` methods. Additionally, `DownloadGetRequestHandler.java` was simplified slightly since the `TusServletResponse` now uniformly handles CRLF removal. Co-authored-by: tomdesair <14034630+tomdesair@users.noreply.github.com> --- .jules/sentinel.md | 4 ++ .../download/DownloadGetRequestHandler.java | 4 +- .../tus/server/util/TusServletResponse.java | 17 ++++-- .../DownloadGetRequestHandlerCRLFTest.java | 59 ------------------- .../util/TusServletResponseCRLFTest.java | 33 +++++++++++ 5 files changed, 52 insertions(+), 65 deletions(-) delete mode 100644 src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerCRLFTest.java create mode 100644 src/test/java/me/desair/tus/server/util/TusServletResponseCRLFTest.java diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 3d73610d..73c52c95 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -15,3 +15,7 @@ **Vulnerability:** A CRLF injection vulnerability was present in `DownloadGetRequestHandler.java` where the unsanitized `info.getFileMimeType()` was passed directly to `servletResponse.setHeader` to form the `Content-Type` header. An attacker could craft a malicious mime-type containing carriage returns and line feeds to inject arbitrary HTTP headers in the response. **Learning:** Similar to `Content-Disposition`, legacy HTTP header fields that accept unencoded strings, like `Content-Type`, are a common vector for HTTP response splitting/CRLF injection when user input is used to construct them. **Prevention:** Always sanitize untrusted input when constructing HTTP headers, especially when not using a built-in framework that automatically sanitizes them. For `Content-Type`, strip out `\r` and `\n` from the unencoded parameter. +## 2026-07-07 - Global CRLF injection in HTTP headers +**Vulnerability:** A CRLF injection vulnerability was identified where unvalidated inputs could be reflected directly into HTTP response headers, leading to HTTP response splitting. +**Learning:** Instead of sanitizing individual headers independently, the overarching wrapper that interacts with the HTTP response output should inherently validate and sanitize the input to prevent injection across all headers. +**Prevention:** `TusServletResponse.java` was modified to include a `sanitizeHeaderValue` method, replacing any instances of `\r` and `\n` characters before interacting with the core `HttpServletResponse`, ensuring consistent CRLF prevention application-wide. 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 6e2e3cfc..5b137715 100644 --- a/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java +++ b/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java @@ -50,12 +50,12 @@ public void process( HttpHeader.CONTENT_DISPOSITION, String.format( CONTENT_DISPOSITION_FORMAT, - info.getFileName().replaceAll("[\r\n\"]", ""), + info.getFileName().replace("\"", ""), URLEncoder.encode(info.getFileName(), StandardCharsets.UTF_8.toString()) .replace("+", "%20"))); servletResponse.setHeader( - HttpHeader.CONTENT_TYPE, info.getFileMimeType().replaceAll("[\r\n]", "")); + HttpHeader.CONTENT_TYPE, info.getFileMimeType()); if (info.hasMetadata()) { servletResponse.setHeader(HttpHeader.UPLOAD_METADATA, info.getEncodedMetadata()); diff --git a/src/main/java/me/desair/tus/server/util/TusServletResponse.java b/src/main/java/me/desair/tus/server/util/TusServletResponse.java index f75ed695..f0e6528e 100644 --- a/src/main/java/me/desair/tus/server/util/TusServletResponse.java +++ b/src/main/java/me/desair/tus/server/util/TusServletResponse.java @@ -41,14 +41,16 @@ public void addDateHeader(String name, long date) { @Override public void setHeader(String name, String value) { - super.setHeader(name, value); - overwriteHeader(name, value); + String sanitizedValue = sanitizeHeaderValue(value); + super.setHeader(name, sanitizedValue); + overwriteHeader(name, sanitizedValue); } @Override public void addHeader(String name, String value) { - super.addHeader(name, value); - recordHeader(name, value); + String sanitizedValue = sanitizeHeaderValue(value); + super.addHeader(name, sanitizedValue); + recordHeader(name, sanitizedValue); } @Override @@ -84,4 +86,11 @@ private void overwriteHeader(String name, String value) { values.add(value); headers.put(name, values); } + + private String sanitizeHeaderValue(String value) { + if (value == null) { + return null; + } + return value.replaceAll("[\r\n]", ""); + } } diff --git a/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerCRLFTest.java b/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerCRLFTest.java deleted file mode 100644 index 49b14d37..00000000 --- a/src/test/java/me/desair/tus/server/download/DownloadGetRequestHandlerCRLFTest.java +++ /dev/null @@ -1,59 +0,0 @@ -package me.desair.tus.server.download; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.mockito.ArgumentMatchers.eq; -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.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class DownloadGetRequestHandlerCRLFTest { - - private DownloadGetRequestHandler handler; - - @Mock private TusServletRequest servletRequest; - @Mock private TusServletResponse servletResponse; - @Mock private UploadStorageService uploadStorageService; - - @Before - public void setUp() { - handler = new DownloadGetRequestHandler(); - } - - @Test - public void testCRLFInjectionInMimeType() throws Exception { - UploadInfo info = new UploadInfo(); - info.setId(new UploadId("test-id")); - info.setLength(10L); - info.setOffset(10L); - // Base64 of "text/html\r\n\r\n" - String maliciousMimeTypeBase64 = - Base64.getEncoder().encodeToString("text/html\r\n\r\n".getBytes()); - info.setEncodedMetadata("filetype " + maliciousMimeTypeBase64); - - when(servletRequest.getRequestURI()).thenReturn("/upload/test"); - when(uploadStorageService.getUploadInfo("/upload/test", "owner")).thenReturn(info); - - handler.process(HttpMethod.GET, servletRequest, servletResponse, uploadStorageService, "owner"); - - ArgumentCaptor contentTypeCaptor = ArgumentCaptor.forClass(String.class); - verify(servletResponse).setHeader(eq(HttpHeader.CONTENT_TYPE), contentTypeCaptor.capture()); - - assertThat(contentTypeCaptor.getValue(), is("text/html")); - } -} diff --git a/src/test/java/me/desair/tus/server/util/TusServletResponseCRLFTest.java b/src/test/java/me/desair/tus/server/util/TusServletResponseCRLFTest.java new file mode 100644 index 00000000..dbf3d31a --- /dev/null +++ b/src/test/java/me/desair/tus/server/util/TusServletResponseCRLFTest.java @@ -0,0 +1,33 @@ +package me.desair.tus.server.util; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletResponse; + +public class TusServletResponseCRLFTest { + private TusServletResponse tusServletResponse; + private MockHttpServletResponse servletResponse; + + @Before + public void setUp() { + servletResponse = new MockHttpServletResponse(); + tusServletResponse = new TusServletResponse(servletResponse); + } + + @Test + public void testCRLFInjectionInSetHeader() { + tusServletResponse.setHeader("TEST", "value\r\nInjected-Header: true"); + assertThat(tusServletResponse.getHeader("TEST"), is("valueInjected-Header: true")); + assertThat(servletResponse.getHeader("TEST"), is("valueInjected-Header: true")); + } + + @Test + public void testCRLFInjectionInAddHeader() { + tusServletResponse.addHeader("TEST", "value\r\nInjected-Header: true"); + assertThat(tusServletResponse.getHeader("TEST"), is("valueInjected-Header: true")); + assertThat(servletResponse.getHeader("TEST"), is("valueInjected-Header: true")); + } +} From 34b83e3cbfc09177cd63c8526279df934e7176c4 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:18:55 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20global=20CRLF=20injection=20in=20HTTP=20headers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces a robust, global defense against HTTP Response Splitting and CRLF Injection vulnerabilities. Instead of patching individual endpoints and headers (such as Content-Disposition or Upload-Metadata) as they arise, `TusServletResponse.java` has been modified to intrinsically sanitize all outgoing header values by stripping carriage return (`\r`) and line feed (`\n`) characters before they are passed to the underlying HttpServletResponse wrapper. The specialized `DownloadGetRequestHandlerCRLFTest.java` was removed and replaced with a broader `TusServletResponseCRLFTest.java` that covers `setHeader` and `addHeader` methods. Additionally, `DownloadGetRequestHandler.java` was simplified slightly since the `TusServletResponse` now uniformly handles CRLF removal. Co-authored-by: tomdesair <14034630+tomdesair@users.noreply.github.com> --- .../desair/tus/server/download/DownloadGetRequestHandler.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 5b137715..493f61f1 100644 --- a/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java +++ b/src/main/java/me/desair/tus/server/download/DownloadGetRequestHandler.java @@ -54,8 +54,7 @@ public void process( URLEncoder.encode(info.getFileName(), StandardCharsets.UTF_8.toString()) .replace("+", "%20"))); - servletResponse.setHeader( - HttpHeader.CONTENT_TYPE, info.getFileMimeType()); + servletResponse.setHeader(HttpHeader.CONTENT_TYPE, info.getFileMimeType()); if (info.hasMetadata()) { servletResponse.setHeader(HttpHeader.UPLOAD_METADATA, info.getEncodedMetadata());