fuzz: fix adding headers in fuzz tests (#8653)

Fixes a logic error in adding headers from a RouteTestCase input to a HeaderMap in route_fuzz_test. Headers were ignored unless they were in the ignore_headers list. This is at least one reason this fuzzer never produced a regex matching crash. With this fixed, a testcase was added that confirms the regex matching crash from CVE-2019-15225 (#8519). Testing: A corpus with a wildcard Regex matcher and a very long URI that produces a crash in the fuzzer. To remove this known crash in the fuzzer, Routes configured with a regex match are explicitly removed. Signed-off-by: Asra Ali <[email protected]>
envoyproxy · Oct 18, 2019 · 8481215 · 8481215
1 parent 87d496d
commit 8481215
Show file tree

Hide file tree

Showing 4 changed files with 326 additions and 11 deletions.
diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc
@@ -1111,5 +1111,12 @@ TEST(HeaderMapImplTest, TestHeaderMapImplyCopy) {
   EXPECT_EQ("bar", baz.get(LowerCaseString("foo"))->value().getStringView());
 }
 
+TEST(HeaderMapImplTest, TestInlineHeaderAdd) {
+  TestHeaderMapImpl foo;
+  foo.addCopy(":path", "GET");
+  EXPECT_EQ(foo.size(), 1);
+  EXPECT_TRUE(foo.Path() != nullptr);
+}
+
 } // namespace Http
 } // namespace Envoy
diff --git a/test/common/router/route_corpus/regex b/test/common/router/route_corpus/regex
diff --git a/test/common/router/route_fuzz_test.cc b/test/common/router/route_fuzz_test.cc
@@ -51,16 +51,25 @@ cleanRouteConfig(envoy::api::v2::RouteConfiguration route_config) {
         // cluster_header from the request header. Because these cluster_headers are destined to be
         // added to a Header Map, we iterate through each route in and remove invalid characters
         // from their cluster headers.
-        std::for_each(virtual_host.mutable_routes()->begin(), virtual_host.mutable_routes()->end(),
-                      [](envoy::api::v2::route::Route& route) {
-                        if (route.has_route()) {
-                          route.mutable_route()->set_cluster_header(
-                              Fuzz::replaceInvalidCharacters(route.route().cluster_header()));
-                          if (route.route().cluster_header().empty()) {
-                            route.mutable_route()->set_cluster_header("not-empty");
-                          }
-                        }
-                      });
+        auto routes = virtual_host.mutable_routes();
+        for (int i = 0; i < routes->size();) {
+          // Erase routes that use a regex matcher. This is deprecated and may cause crashes when
+          // wildcards are matched against very long headers.
+          // See https://github.com/envoyproxy/envoy/issues/7728.
+          if (routes->Get(i).match().path_specifier_case() ==
+              envoy::api::v2::route::RouteMatch::kRegex) {
+            routes->erase(routes->begin() + i);
+          } else {
+            if (routes->Get(i).has_route()) {
+              routes->Mutable(i)->mutable_route()->set_cluster_header(
+                  Fuzz::replaceInvalidCharacters(routes->Mutable(i)->route().cluster_header()));
+              if (routes->Get(i).route().cluster_header().empty()) {
+                routes->Mutable(i)->mutable_route()->set_cluster_header("not-empty");
+              }
+            }
+            ++i;
+          }
+        }
       });
 
   return clean_config;

diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h
@@ -82,7 +82,7 @@ inline Http::TestHeaderMapImpl fromHeaders(
     // not supposed to do this.
     const std::string key =
         header.key().empty() ? "not-empty" : replaceInvalidCharacters(header.key());
-    if (ignore_headers.find(StringUtil::toLower(key)) != ignore_headers.end()) {
+    if (ignore_headers.find(StringUtil::toLower(key)) == ignore_headers.end()) {
       header_map.addCopy(key, replaceInvalidCharacters(header.value()));
     }
   }