ubeddulla commented on code in PR #3387:
URL: https://github.com/apache/brpc/pull/3387#discussion_r3586192032
##########
src/brpc/details/http_message.cpp:
##########
@@ -578,6 +578,16 @@ 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.
+static bool HeaderHasCRLF(const std::string& s) {
+ return s.find_first_of("\r\n") != std::string::npos;
Review Comment:
Good point. I put it behind -http_check_outbound_header_crlf (default on).
The scan is a single find_first_of over the header name/value, so it's cheap,
but anyone who never forwards untrusted header values can turn it off.
##########
src/brpc/details/http_message.cpp:
##########
@@ -644,12 +654,17 @@ void MakeRawHttpRequest(butil::IOBuf* request,
}
os << BRPC_CRLF;
}
- if (!h->content_type().empty()) {
+ if (!h->content_type().empty() && !HeaderHasCRLF(h->content_type())) {
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(ERROR) << "Skip header `" << it->first
Review Comment:
Done, switched both to LOG(WARNING). Also wrapped the logged name in
butil::ToPrintable so a CR/LF in the name can't inject newlines into the log.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
