Guard PacketFieldHandle index against uint16_t overflow

netkat/BUILD.bazel

@@ -250,6 +250,7 @@ cc_library(
     deps = [
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/log",
+        "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/strings:string_view",

netkat/packet_field.cc

@@ -14,8 +14,11 @@
 
 #include "netkat/packet_field.h"
 
+#include <cstdint>
+#include <limits>
 #include <string>
 
+#include "absl/log/check.h"
 #include "absl/log/log.h"
 #include "absl/status/status.h"
 #include "absl/strings/string_view.h"
@@ -25,6 +28,16 @@ namespace netkat {
 
 PacketFieldHandle PacketFieldManager::GetOrCreatePacketFieldHandle(
     absl::string_view field_name) {
+  // `PacketFieldHandle` packs its index into a 16-bit field (see
+  // packet_field.h), so silently truncating an index that no longer fits
+  // would let two distinct field names collide on the same handle. Fail
+  // loudly instead, since that's a violation of a documented, load-bearing
+  // assumption rather than a recoverable runtime condition.
+  CHECK_LE(field_names_.size(),  // Crash OK
+           std::numeric_limits<uint16_t>::max())
+      << "PacketFieldManager: exceeded the maximum of "
+      << std::numeric_limits<uint16_t>::max() + 1 << " distinct packet fields.";
+
   auto [it, inserted] = packet_field_by_name_.try_emplace(
       field_name, PacketFieldHandle(field_names_.size()));
   if (inserted) field_names_.push_back(std::string(field_name));

netkat/packet_field_test.cc

@@ -75,5 +75,26 @@ TEST(PacketFieldManagerTest, GetFieldNameReturnsNameOfPacketFieldHandle) {
   EXPECT_EQ(Manager().GetFieldName(foo), "foo");
   EXPECT_EQ(Manager().GetFieldName(bar), "bar");
 }
+
+// `PacketFieldHandle` packs its index into 16 bits (see packet_field.h), so
+// the manager must reject attempts to intern more than 2^16 distinct field
+// names instead of silently truncating the index and aliasing two different
+// field names onto the same handle.
+//
+// Uses a manager local to this test (rather than the shared `Manager()`
+// above) so as to not pollute global test state with tens of thousands of
+// interned fields.
+TEST(PacketFieldManagerTest,
+     GetOrCreatePacketFieldHandleCrashesOnTooManyDistinctFields) {
+  PacketFieldManager manager;
+  constexpr int kMaxDistinctFields = 1 << 16;
+  for (int i = 0; i < kMaxDistinctFields; ++i) {
+    (void)manager.GetOrCreatePacketFieldHandle(absl::StrCat("field_", i));
+  }
+  EXPECT_DEATH(
+      (void)manager.GetOrCreatePacketFieldHandle(
+          absl::StrCat("field_", kMaxDistinctFields)),
+      "exceeded the maximum");
+}
 }  // namespace
 }  // namespace netkat