HTTP/2 Check header names in HeaderCoalescer

net/spdy/header_coalescer.cc

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/header_coalescer.h"
 
 #include <utility>
 
 #include "base/strings/string_util.h"
+#include "net/http/http_util.h"
 #include "net/spdy/platform/api/spdy_estimate_memory_usage.h"
 
 namespace net {
 
 const size_t kMaxHeaderListSize = 256 * 1024;
 
 void HeaderCoalescer::OnHeader(base::StringPiece key, base::StringPiece value) {
   if (error_seen_) {
     return;
   }
 
   if (key.empty()) {
     DVLOG(1) << "Header name must not be empty.";
     error_seen_ = true;
     return;
   }
 
+  if (!HttpUtil::IsValidHeaderName(key)) {
+    error_seen_ = true;
+    return;
+  }
+
   // 32 byte overhead according to RFC 7540 Section 6.5.2.
   header_list_size_ += key.size() + value.size() + 32;
   if (header_list_size_ > kMaxHeaderListSize) {
     error_seen_ = true;
     return;
   }
 
   if (key[0] == ':') {

[Biren Roy, 2017/02/22 20:26:23]

This code block appears intended to handle pseudo-header fields. If
pseudo-header fields are consumed by another layer, it may be possible to remove
this code block and any associated code paths.

(Could be done as a future clean-up change.)

[xunjieli, 2017/02/22 22:19:58]

I did a brief scan of the spdy code, but I didn't any pseudo-header handling
like this anywhere else. I might not be looking at the right places.
Bence, do you know?

[Biren Roy, 2017/02/22 22:34:20]

Yeah, I think you're right. Nothing before this layer will consume
pseudo-headers.

It looks like you will have to explicitly allow a leading ':' in the conditional
above. One easy way to do that might be to copy |key| and remove a leading ':',
if present, before passing the StringPiece to HttpUtils::IsValidHeaderName().

[xunjieli, 2017/02/23 16:10:42]

Done. Good catch!

     if (regular_header_seen_) {
       error_seen_ = true;
       return;
     }
   } else {
     regular_header_seen_ = true;
   }
 
   // End of line delimiter is forbidden according to RFC 7230 Section 3.2.
   // Line folding, RFC 7230 Section 3.2.4., is a special case of this.
@@ skipping 24 matching lines @@
   DCHECK(headers_valid_);
   headers_valid_ = false;
   return std::move(headers_);
 }
 
 size_t HeaderCoalescer::EstimateMemoryUsage() const {
   return SpdyEstimateMemoryUsage(headers_);
 }
 
 }  // namespace net