This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 1665ebed Skip header fields containing CR/LF in http serialization
(#3387)
1665ebed is described below
commit 1665ebedfd767f72d9f46ebe3241037221e6dd27
Author: UB <[email protected]>
AuthorDate: Mon Aug 24 13:28:48 2026 +0530
Skip header fields containing CR/LF in http serialization (#3387)
* skip header fields containing CR/LF in http serialization
* Gate CR/LF header check behind a flag; log at WARNING with sanitized name
Signed-off-by: ubeddulla khan <[email protected]>
* Log the skipped header value alongside its name
Signed-off-by: ubeddulla khan <[email protected]>
* Use separate IOBufs for request and response in the CR/LF header test
Signed-off-by: ubeddulla khan <[email protected]>
---------
Signed-off-by: ubeddulla khan <[email protected]>
---
src/brpc/details/http_message.cpp | 48 +++++++++++++++++++++++++++++++---
test/brpc_http_message_unittest.cpp | 52 +++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/src/brpc/details/http_message.cpp
b/src/brpc/details/http_message.cpp
index 13beb67a..45f77bab 100644
--- a/src/brpc/details/http_message.cpp
+++ b/src/brpc/details/http_message.cpp
@@ -23,6 +23,7 @@
#include "butil/scoped_lock.h"
#include "butil/endpoint.h"
#include "butil/base64.h"
+#include "butil/binary_printer.h" // ToPrintable
#include "bthread/bthread.h" // bthread_usleep
#include "brpc/log.h"
#include "brpc/reloadable_flags.h"
@@ -38,6 +39,9 @@ DEFINE_bool(http_verbose, false,
"[DEBUG] Print EVERY http request/response");
DEFINE_int32(http_verbose_max_body_length, 512,
"[DEBUG] Max body length printed when -http_verbose is on");
+DEFINE_bool(http_check_outbound_header_crlf, true,
+ "Skip outbound http header fields whose name or value contains "
+ "CR/LF to prevent request/response splitting.");
DECLARE_int64(socket_max_unwritten_bytes);
DECLARE_uint64(max_body_size);
@@ -578,6 +582,18 @@ std::ostream& operator<<(std::ostream& os, const
http_parser& parser) {
#define BRPC_CRLF "\r\n"
+// A header field-name or field-value carrying a raw CR or LF lets whoever
+// controls it close the current line and inject extra header fields (or a
+// body) into the serialized message, i.e. HTTP request/response splitting.
+// The inbound parser already refuses these bytes; the outbound path drops
+// such fields so a value forwarded from an untrusted source can't smuggle
+// headers. Gated by -http_check_outbound_header_crlf so it can be turned
+// off on hot paths that never forward untrusted header values.
+static bool HeaderHasCRLF(const std::string& s) {
+ return FLAGS_http_check_outbound_header_crlf &&
+ s.find_first_of("\r\n") != std::string::npos;
+}
+
// Request format
// Request = Request-Line ; Section 5.1
// *(( general-header ; Section 4.5
@@ -645,11 +661,23 @@ void MakeRawHttpRequest(butil::IOBuf* request,
os << BRPC_CRLF;
}
if (!h->content_type().empty()) {
- os << "Content-Type: " << h->content_type()
- << BRPC_CRLF;
+ if (HeaderHasCRLF(h->content_type())) {
+ LOG(WARNING) << "Skip Content-Type `"
+ << butil::ToPrintable(h->content_type())
+ << "' containing CR/LF to avoid injection";
+ } else {
+ os << "Content-Type: " << h->content_type()
+ << BRPC_CRLF;
+ }
}
for (HttpHeader::HeaderIterator it = h->HeaderBegin();
it != h->HeaderEnd(); ++it) {
+ if (HeaderHasCRLF(it->first) || HeaderHasCRLF(it->second)) {
+ LOG(WARNING) << "Skip header `" << butil::ToPrintable(it->first)
+ << ": " << butil::ToPrintable(it->second)
+ << "' containing CR/LF to avoid injection";
+ continue;
+ }
os << it->first << ": " << it->second << BRPC_CRLF;
}
if (h->GetHeader("Accept") == nullptr) {
@@ -734,11 +762,23 @@ void MakeRawHttpResponse(butil::IOBuf* response,
}
}
if (!is_invalid_content && !h->content_type().empty()) {
- os << "Content-Type: " << h->content_type()
- << BRPC_CRLF;
+ if (HeaderHasCRLF(h->content_type())) {
+ LOG(WARNING) << "Skip Content-Type `"
+ << butil::ToPrintable(h->content_type())
+ << "' containing CR/LF to avoid injection";
+ } else {
+ os << "Content-Type: " << h->content_type()
+ << BRPC_CRLF;
+ }
}
for (HttpHeader::HeaderIterator it = h->HeaderBegin();
it != h->HeaderEnd(); ++it) {
+ if (HeaderHasCRLF(it->first) || HeaderHasCRLF(it->second)) {
+ LOG(WARNING) << "Skip header `" << butil::ToPrintable(it->first)
+ << ": " << butil::ToPrintable(it->second)
+ << "' containing CR/LF to avoid injection";
+ continue;
+ }
os << it->first << ": " << it->second << BRPC_CRLF;
}
os << BRPC_CRLF; // CRLF before content
diff --git a/test/brpc_http_message_unittest.cpp
b/test/brpc_http_message_unittest.cpp
index bd2ac5ed..87f3b373 100644
--- a/test/brpc_http_message_unittest.cpp
+++ b/test/brpc_http_message_unittest.cpp
@@ -703,6 +703,58 @@ TEST(HttpMessageTest, serialize_http_response) {
<< butil::ToPrintable(response);
}
+TEST(HttpMessageTest, serialize_header_with_crlf_is_not_injected) {
+ // A header value carrying CR/LF must not terminate the current line and
+ // introduce extra header fields (HTTP request/response splitting).
+ butil::EndPoint ep;
+ ASSERT_EQ(0, butil::str2endpoint("127.0.0.1:1234", &ep));
+
+ brpc::HttpHeader req_header;
+ req_header.set_method(brpc::HTTP_METHOD_POST);
+ req_header.SetHeader("X-Evil", "a\r\nInjected: 1");
+ butil::IOBuf req_content;
+ req_content.append("data");
+ butil::IOBuf request;
+ MakeRawHttpRequest(&request, &req_header, ep, &req_content);
+ std::string request_str = request.to_string();
+ ASSERT_EQ(std::string::npos, request_str.find("Injected: 1")) <<
request_str;
+
+ brpc::HttpHeader res_header;
+ res_header.SetHeader("X-Evil", "a\r\nInjected: 1");
+ butil::IOBuf res_content;
+ res_content.append("data");
+ butil::IOBuf response;
+ MakeRawHttpResponse(&response, &res_header, &res_content);
+ std::string response_str = response.to_string();
+ ASSERT_EQ(std::string::npos, response_str.find("Injected: 1")) <<
response_str;
+}
+
+TEST(HttpMessageTest, serialize_content_type_with_crlf_is_not_injected) {
+ // Content-Type goes through the same emission path and must be dropped
+ // (not written) when it carries CR/LF.
+ butil::EndPoint ep;
+ ASSERT_EQ(0, butil::str2endpoint("127.0.0.1:1234", &ep));
+
+ brpc::HttpHeader req_header;
+ req_header.set_method(brpc::HTTP_METHOD_POST);
+ req_header.set_content_type("text/plain\r\nInjected: 1");
+ butil::IOBuf req_content;
+ req_content.append("data");
+ butil::IOBuf request;
+ MakeRawHttpRequest(&request, &req_header, ep, &req_content);
+ std::string request_str = request.to_string();
+ ASSERT_EQ(std::string::npos, request_str.find("Injected: 1")) <<
request_str;
+
+ brpc::HttpHeader res_header;
+ res_header.set_content_type("text/plain\r\nInjected: 1");
+ butil::IOBuf res_content;
+ res_content.append("data");
+ butil::IOBuf response;
+ MakeRawHttpResponse(&response, &res_header, &res_content);
+ std::string response_str = response.to_string();
+ ASSERT_EQ(std::string::npos, response_str.find("Injected: 1")) <<
response_str;
+}
+
TEST(HttpMessageTest, http_1_1_request_without_host) {
brpc::FLAGS_allow_http_1_1_request_without_host = false;
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
