From 77fd332c80012da3485884b1b1b0b83fea42a940 Mon Sep 17 00:00:00 2001 From: abhyuday-fr Date: Tue, 14 Jul 2026 09:37:05 +0530 Subject: [PATCH] Guard PacketFieldHandle index against uint16_t overflow PacketFieldHandle packs its index into 16 bits, but GetOrCreatePacketFieldHandle had no check that field_names_.size() still fits, so interning more than 2^16 distinct field names would silently truncate the index and alias two different field names onto the same handle. Add an explicit CHECK and a regression test. Fixes #123 --- netkat/BUILD.bazel | 1 + netkat/packet_field.cc | 13 +++++++++++++ netkat/packet_field_test.cc | 21 +++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/netkat/BUILD.bazel b/netkat/BUILD.bazel index 89b605d..cb2ec61 100644 --- a/netkat/BUILD.bazel +++ b/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", diff --git a/netkat/packet_field.cc b/netkat/packet_field.cc index 565783f..261520c 100644 --- a/netkat/packet_field.cc +++ b/netkat/packet_field.cc @@ -14,8 +14,11 @@ #include "netkat/packet_field.h" +#include +#include #include +#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::max()) + << "PacketFieldManager: exceeded the maximum of " + << std::numeric_limits::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)); diff --git a/netkat/packet_field_test.cc b/netkat/packet_field_test.cc index 50222ca..63d3a44 100644 --- a/netkat/packet_field_test.cc +++ b/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