nghttpx: Replace \r\n with space when constructing HTTP/1 headers

src/http2.cc

@@ -90,6 +90,22 @@ void capitalize(std::string& s, size_t offset)
   }
 }
 
+bool check_header_value(const char *value)
+{
+  return strpbrk(value, "\r\n") == nullptr;
+}
+
+bool check_header_value(const nghttp2_nv* nv)
+{
+  size_t i;
+  for(i = 0; i < nv->valuelen; ++i) {
+    if(nv->value[i] == '\r' || nv->value[i] == '\n') {
+      return false;
+    }
+  }
+  return true;
+}
+
 void sanitize_header_value(std::string& s, size_t offset)
 {
   for(size_t i = offset, eoi = s.size(); i < eoi; ++i) {
@@ -290,6 +306,7 @@ void build_http1_headers_from_norm_headers
       capitalize(hdrs, hdrs.size()-headers[i].first.size());
       hdrs += ": ";
       hdrs += headers[i].second;
+      sanitize_header_value(hdrs, hdrs.size() - headers[i].second.size());
       hdrs += "\r\n";
       ++i;
     } else if(rv > 0) {
@@ -303,6 +320,7 @@ void build_http1_headers_from_norm_headers
     capitalize(hdrs, hdrs.size()-headers[i].first.size());
     hdrs += ": ";
     hdrs += headers[i].second;
+    sanitize_header_value(hdrs, hdrs.size() - headers[i].second.size());
     hdrs += "\r\n";
   }
 }

src/http2.h

@@ -42,6 +42,12 @@ const char* get_status_string(int status_code);
 
 void capitalize(std::string& s, size_t offset);
 
+// Returns false if |value| contains \r or \n.
+bool check_header_value(const char *value);
+
+// Returns false if |nv->value| contains \r or \n.
+bool check_header_value(const nghttp2_nv *nv);
+
 void sanitize_header_value(std::string& s, size_t offset);
 
 // Copies the |field| component value from |u| and |url| to the

src/http2_test.cc

@@ -162,6 +162,26 @@ void test_http2_build_http1_headers_from_norm_headers(void)
             "Te: 8\r\n"
             "Te: 9\r\n"
             "Zulu: 12\r\n");
+
+  hdrs.clear();
+  auto hd2 = std::vector<std::pair<std::string, std::string>>
+    {{"alpha", "bravo\r\ncharlie\r\n"}};
+  http2::build_http1_headers_from_norm_headers(hdrs, hd2);
+  CU_ASSERT(hdrs == "Alpha: bravo  charlie  \r\n");
+}
+
+void test_http2_check_header_value(void)
+{
+  CU_ASSERT(http2::check_header_value("alpha"));
+  CU_ASSERT(!http2::check_header_value("alpha\r"));
+  CU_ASSERT(!http2::check_header_value("alpha\n"));
+
+  nghttp2_nv nv1 = MAKE_NV("alpha", "bravo");
+  CU_ASSERT(http2::check_header_value(&nv1));
+  nghttp2_nv nv2 = MAKE_NV("alpha", "bravo\r");
+  CU_ASSERT(!http2::check_header_value(&nv2));
+  nghttp2_nv nv3 = MAKE_NV("alpha", "bravo\n");
+  CU_ASSERT(!http2::check_header_value(&nv3));
 }
 
 } // namespace shrpx

src/http2_test.h

@@ -33,6 +33,7 @@ void test_http2_get_header(void);
 void test_http2_value_lws(void);
 void test_http2_copy_norm_headers_to_nv(void);
 void test_http2_build_http1_headers_from_norm_headers(void);
+void test_http2_check_header_value(void);
 
 } // namespace shrpx
 

src/shrpx-unittest.cc

@@ -81,6 +81,8 @@ int main(int argc, char* argv[])
                    shrpx::test_http2_copy_norm_headers_to_nv) ||
       !CU_add_test(pSuite, "http2_build_http1_headers_from_norm_headers",
                    shrpx::test_http2_build_http1_headers_from_norm_headers) ||
+      !CU_add_test(pSuite, "http2_check_header_value",
+                   shrpx::test_http2_check_header_value) ||
       !CU_add_test(pSuite, "downstream_normalize_request_headers",
                    shrpx::test_downstream_normalize_request_headers) ||
       !CU_add_test(pSuite, "downstream_normalize_response_headers",

src/shrpx_http2_upstream.cc

@@ -231,7 +231,11 @@ int on_frame_recv_callback
     if(!host || !path || !method ||
        http2::value_lws(host) || http2::value_lws(path) ||
        http2::value_lws(method) ||
-       (!is_connect && (!scheme || http2::value_lws(scheme)))) {
+       (!is_connect && (!scheme || http2::value_lws(scheme))) ||
+       !http2::check_header_value(host) ||
+       !http2::check_header_value(path) ||
+       !http2::check_header_value(method) ||
+       (scheme && !http2::check_header_value(scheme))) {
       upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR);
       return 0;
     }

src/shrpx_http_downstream_connection.cc

@@ -114,11 +114,11 @@ int HttpDownstreamConnection::attach_downstream(Downstream *downstream)
 
 int HttpDownstreamConnection::push_request_headers()
 {
+  // Assume that method and request path do not contain \r\n.
   std::string hdrs = downstream_->get_request_method();
   hdrs += " ";
   hdrs += downstream_->get_request_path();
-  hdrs += " ";
-  hdrs += "HTTP/1.1\r\n";
+  hdrs += " HTTP/1.1\r\n";
   downstream_->normalize_request_headers();
   auto end_headers = std::end(downstream_->get_request_headers());
   http2::build_http1_headers_from_norm_headers
@@ -132,6 +132,7 @@ int HttpDownstreamConnection::push_request_headers()
     hdrs += "X-Forwarded-For: ";
     if(xff != end_headers) {
       hdrs += (*xff).second;
+      http2::sanitize_header_value(hdrs, hdrs.size() - (*xff).second.size());
       hdrs += ", ";
     }
     hdrs += downstream_->get_upstream()->get_client_handler()->get_ipaddr();
@@ -139,22 +140,23 @@ int HttpDownstreamConnection::push_request_headers()
   } else if(xff != end_headers) {
     hdrs += "X-Forwarded-For: ";
     hdrs += (*xff).second;
+    http2::sanitize_header_value(hdrs, hdrs.size() - (*xff).second.size());
     hdrs += "\r\n";
   }
   if(downstream_->get_request_method() != "CONNECT") {
     hdrs += "X-Forwarded-Proto: ";
     if(util::istartsWith(downstream_->get_request_path(), "http:")) {
-      hdrs += "http";
+      hdrs += "http\r\n";
     } else {
-      hdrs += "https";
+      hdrs += "https\r\n";
     }
-    hdrs += "\r\n";
   }
   auto expect = downstream_->get_norm_request_header("expect");
   if(expect != end_headers &&
      !util::strifind((*expect).second.c_str(), "100-continue")) {
     hdrs += "Expect: ";
     hdrs += (*expect).second;
+    http2::sanitize_header_value(hdrs, hdrs.size() - (*expect).second.size());
     hdrs += "\r\n";
   }
   auto via = downstream_->get_norm_request_header("via");
@@ -162,12 +164,14 @@ int HttpDownstreamConnection::push_request_headers()
     if(via != end_headers) {
       hdrs += "Via: ";
       hdrs += (*via).second;
+      http2::sanitize_header_value(hdrs, hdrs.size() - (*via).second.size());
       hdrs += "\r\n";
     }
   } else {
     hdrs += "Via: ";
     if(via != end_headers) {
       hdrs += (*via).second;
+      http2::sanitize_header_value(hdrs, hdrs.size() - (*via).second.size());
       hdrs += ", ";
     }
     hdrs += http::create_via_header_value(downstream_->get_request_major(),

src/shrpx_https_upstream.cc

@@ -684,12 +684,14 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream)
     if(via != end_headers) {
       hdrs += "Via: ";
       hdrs += (*via).second;
+      http2::sanitize_header_value(hdrs, hdrs.size() - (*via).second.size());
       hdrs += "\r\n";
     }
   } else {
     hdrs += "Via: ";
     if(via != end_headers) {
       hdrs += (*via).second;
+      http2::sanitize_header_value(hdrs, hdrs.size() - (*via).second.size());
       hdrs += ", ";
     }
     hdrs += http::create_via_header_value

src/shrpx_spdy_upstream.cc

@@ -180,7 +180,11 @@ void on_ctrl_recv_callback
         downstream->add_request_header(nv[i], nv[i+1]);
       }
     }
-    if(!path || !host || !method) {
+    if(!path || !host || !method ||
+       !http2::check_header_value(host) ||
+       !http2::check_header_value(path) ||
+       !http2::check_header_value(method) ||
+       (scheme && !http2::check_header_value(scheme))) {
       upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
       return;
     }