http: fix allocation bug introduced in envoyproxy#4211.

There were some non-local invariants that header_map_impl_fuzz_test surfaced around minimum dynamic
buffer size. This PR improves comments and documentation of invariants and fixes the allocation issue
to maintain them.

Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10038.

Risk level: Low. It's recommended to bump to this for potential security reasons if you are already post
  envoyproxy#4211.
Testing: Unit test and corpus entry added.

Signed-off-by: Harvey Tuch <htuch@google.com>

include/envoy/http/header_map.h

@@ -155,10 +155,13 @@ class HeaderString {
 
 private:
   union {
+    // This should reference inline_buffer_ for Type::Inline.
     char* dynamic_;
     const char* ref_;
   } buffer_;
 
+  // Capacity in both Type::Inline and Type::Dynamic cases must be at least MinDynamicCapacity in
+  // header_map_impl.cc.
   union {
     char inline_buffer_[128];
     uint32_t dynamic_capacity_;

source/common/http/header_map_impl.cc

@@ -14,9 +14,17 @@
 namespace Envoy {
 namespace Http {
 
+namespace {
+constexpr size_t MinDynamicCapacity{32};
+// This includes the NULL (StringUtil::itoa technically only needs 21).
+constexpr size_t MaxIntegerLength{32};
+} // namespace
+
 HeaderString::HeaderString() : type_(Type::Inline) {
   buffer_.dynamic_ = inline_buffer_;
   clear();
+  static_assert(sizeof(inline_buffer_) >= MaxIntegerLength, "");
+  static_assert(MinDynamicCapacity >= MaxIntegerLength, "");
 }
 
 HeaderString::HeaderString(const LowerCaseString& ref_value) : type_(Type::Reference) {
@@ -70,7 +78,8 @@ void HeaderString::append(const char* data, uint32_t size) {
     // Rather than be too clever and optimize this uncommon case, we dynamically
     // allocate and copy.
     type_ = Type::Dynamic;
-    dynamic_capacity_ = (string_length_ + size) * 2;
+    dynamic_capacity_ =
+        std::max(MinDynamicCapacity, static_cast<size_t>((string_length_ + size) * 2));
     char* buf = static_cast<char*>(malloc(dynamic_capacity_));
     RELEASE_ASSERT(buf != nullptr, "");
     memcpy(buf, buffer_.ref_, string_length_);
@@ -94,6 +103,7 @@ void HeaderString::append(const char* data, uint32_t size) {
       // If the resizing will cause buffer overflow due to hitting uint32_t::max, an OOM is likely
       // imminent. Fast-fail rather than allow a buffer overflow attack (issue #1421)
       RELEASE_ASSERT(new_capacity <= std::numeric_limits<uint32_t>::max(), "");
+      ASSERT(new_capacity >= MinDynamicCapacity);
       buffer_.dynamic_ = static_cast<char*>(malloc(new_capacity));
       memcpy(buffer_.dynamic_, inline_buffer_, string_length_);
       RELEASE_ASSERT(buffer_.dynamic_ != nullptr, "");
@@ -182,8 +192,13 @@ void HeaderString::setInteger(uint64_t value) {
   }
 
   case Type::Inline:
+    // buffer_.dynamic_ should always point at inline_buffer_ for Type::Inline.
+    ASSERT(buffer_.dynamic_ == inline_buffer_);
   case Type::Dynamic: {
     // Whether dynamic or inline the buffer is guaranteed to be large enough.
+    ASSERT(dynamic_capacity_ >= MaxIntegerLength);
+    // It's safe to use buffer.dynamic_, since buffer.ref_ is union aliased.
+    ASSERT(&buffer_.dynamic_ == &buffer_.ref_);
     string_length_ = StringUtil::itoa(buffer_.dynamic_, 32, value);
   }
   }

test/common/http/header_map_impl_corpus/clusterfuzz-testcase-minimized-header_map_impl_fuzz_test-5689833624698880

@@ -0,0 +1 @@
+actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions {   set_reference {   } } actions { } actions {   get_and_mutate {     append: ""   } } actions { } actions {   get_and_mutate {     set_integer: 0   } } actions { } actions { } actions { } actions { } actions { } actions { } actions { } actions { } 

test/common/http/header_map_impl_test.cc

@@ -692,6 +692,15 @@ TEST(HeaderMapImplTest, TestAppendHeader) {
     HeaderMapImpl::appendToHeader(value3, "");
     EXPECT_EQ(value3, "empty");
   }
+  // Regression test for appending to an empty string with empty, then setting integer.
+  {
+    const std::string empty;
+    HeaderString value4(empty);
+    HeaderMapImpl::appendToHeader(value4, " ");
+    value4.setInteger(0);
+    EXPECT_STREQ("0", value4.c_str());
+    EXPECT_EQ(1U, value4.size());
+  }
 }
 
 TEST(HeaderMapImplTest, PseudoHeaderOrder) {