[Security] Prevent HTTP Response Header Injection by sanitizing CR/LF characters

[notfresh]

for the bug issue:

#1165

---

Title

[Security] Prevent HTTP Response Header Injection by sanitizing CR/LF characters

🚨 Summary

This PR introduces a critical security fix to prevent HTTP Response Header Injection (also known as CRLF Injection). It ensures that any Carriage Return (\r) or Line Feed (\n) characters are stripped from header names and values before they are stored or sent in an HTTP response.

📖 Background: The Vulnerability

In the current implementation, the crow::response and crow::request objects allow setting header keys and values directly from user input without validation. Since the HTTP protocol uses \r\n to delimit headers, an attacker who can control a header value can inject these characters to:

1. Inject Malicious Headers: e.g., injecting a fake Set-Cookie header to hijack sessions.
2. Perform Response Splitting: Terminate the header section prematurely (\r\n\r\n) and inject arbitrary content into the response body, leading to Cross-Site Scripting (XSS) attacks.
3. Bypass Security Controls: Inject headers that might confuse downstream proxies or caches.

Proof of Concept (PoC)

Before this fix, the following code would result in a malformed and dangerous HTTP response:

// Attacker controls the value "safe\r\nInjected-Header: malicious"
res.set_header("X-Custom", "safe\r\nInjected-Header: malicious");

Resulting Raw HTTP Response (Vulnerable):

HTTP/1.1 200 OK
X-Custom: safe
Injected-Header: malicious
...

The browser interprets Injected-Header as a legitimate new header controlled by the attacker.

🛠️ Solution Strategy

This fix implements a sanitization strategy at the entry point of all header modification methods. Instead of throwing an exception (which could break existing applications unexpectedly), we silently strip invalid characters. This approach:

• Neutralizes the threat: Converts malicious input into harmless text.
• Maintains Compatibility: Adheres to RFC 7230, which states that header fields should not contain control characters like CR or LF.
• Zero Overhead: Uses efficient in-place string manipulation (std::remove_if).

🔑 Key Changes

1. New Utility Function (include/crow/http_request.h)

Added an inline helper function sanitize_header_value that removes \r and \n from a given string.

/// Remove CR (\r) and LF (\n) characters from a header name or value to prevent header injection. 
inline void sanitize_header_value(std::string& s)
{
    s.erase(std::remove_if(s.begin(), s.end(),
                           [](char c) { return c == '\r' || c == '\n'; }),
            s.end());
}

2. Protection in Response Handlers (include/crow/http_response.h)

Applied sanitization to both set_header and add_header methods in the response class.

• set_header: Cleans key and value before updating the map.
• add_header: Cleans key and value before insertion.

3. Protection in Request Handlers (include/crow/http_request.h)

Applied sanitization to the add_header method in the request class to prevent internal header pollution if requests are forwarded or processed internally.

4. New Example/Test Case (examples/helloworld_inject.cpp)

Added a new example demonstrating the fix.

• Scenario: Attempts to set a header with embedded \r\n.
• Expected Behavior: The output shows the characters removed, merging the content into a single line value, preventing the creation of a new header line.
• Build Integration: Updated examples/CMakeLists.txt to include helloworld_inject.

🧪 Testing & Verification

Manual Verification

1. Build the new example:

   mkdir build && cd build
   cmake .. 
   make helloworld_inject
   ./helloworld_inject

2. Send a request using curl in verbose mode:

   curl -v http://localhost:18080/

3. Observe:
   • Before Fix: You would see two distinct headers: X-Custom and Injected.
   • After Fix: You will see only one header X-Custom with the value safeInjected: yes. The newline is gone, and no new header is created.

Automated Testing Recommendation

It is recommended to add a unit test in tests/unittest.cpp:

• Create a crow::response object.
• Call set_header with a value containing \r\n.
• Assert that the stored value does not contain \r or \n.
• Assert that the generated raw HTTP string does not contain unexpected newlines within the header section.

📉 Impact Analysis

• Security: High. Fixes CWE-113 (Improper Neutralization of CRLF Sequences in HTTP Headers).
• Performance: Negligible. The sanitization involves a single pass over the string only when headers are set.
• Breaking Changes: Low/Risk-Free. Any code relying on injecting newlines into headers was technically violating HTTP standards and behaving insecurely. This change forces compliance with RFC 7230.

[notfresh]

Why Filtering on the Request Side Has Limited Impact

1. The Nature of Request Headers
Request headers originate directly from the client. An attacker already has full control over their own request headers; they do not need to inject \r\n characters to manipulate them. The HTTP parser (see parser.h:49) strictly delimits headers based on \r\n. Consequently, it is impossible for an attacker to inject cross-line content into a single header value during the parsing phase.

2. Usage Scope of req.add_header()
The req.add_header() method is primarily used for testing and internal framework logic (as evidenced by its call sites being exclusively in unittest.cpp). It is not invoked during the parsing of incoming network requests. The actual parsing of request headers follows a path in parser.h that directly emplaces entries into the headers map, completely bypassing add_header().

Why It Is Still Worth Retaining

1. Defense in Depth
Even if the network parser is safe, filtering provides a crucial safety net for application logic. If developers manually call req.add_header() within middleware or routes using untrusted data as a value, and this value is subsequently:

• Reflected into response headers,
• Written to application logs, or
• Forwarded in downstream HTTP requests,
  then sanitizing \r\n prevents Log Injection and Secondary Injection attacks.

2. Negligible Cost
The performance overhead of std::remove_if is minimal and does not impact overall system throughput.

The Real Gap (and Why It's Not a Vulnerability)

It is important to note that in parser.h:49, raw request headers are parsed and directly emplaced into req.headers without passing through sanitize_header_value.

However, this is not a security issue. The HTTP parser itself guarantees that each header field and value is already strictly delimited by lines. Since the protocol structure prevents newlines from existing within a parsed value, additional sanitization at the parser level is redundant for preventing header injection via network traffic.

[notfresh]

@gittiver could you please check this pr?