🛡️ Sentinel: [HIGH] Fix global CRLF injection in HTTP headers#107
Conversation
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, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
coverage: 94.749% (-0.06%) from 94.813% — jules-12850832640110649913-688d6719 into master |
|
@jules, can you run the code formatting step that is defined in the Maven pom.xml file? Or you can use the pre-commit hooks. |
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>
2 participants
🚨 Severity: HIGH
💡 Vulnerability: CRLF Injection (HTTP Response Splitting) was possible when unsanitized user inputs or generic properties were placed into HTTP response headers (such as
Upload-MetadataorContent-Disposition). This allowed malicious actors to inject arbitrary headers and potentially modify the response body payload by appending\r\n.🎯 Impact: Attackers could manipulate server responses, launch XSS attacks, deface pages, or poison web caches by injecting fake or malicious headers.
🔧 Fix: Rather than addressing each affected header independently, the overarching
TusServletResponseclass was modified with asanitizeHeaderValuemethod. This method automatically strips\rand\ncharacters from all header values whensetHeaderoraddHeaderis called, offering global CRLF protection.✅ Verification: Ensure tests pass via
mvn test. The newTusServletResponseCRLFTestverifies that CRLF characters injected intosetHeaderandaddHeaderare successfully stripped.PR created automatically by Jules for task 12850832640110649913 started by @tomdesair