From a9b649eb05d726b4eec3b84b80d43b1f8fc58771 Mon Sep 17 00:00:00 2001 From: Deepesh Pathak Date: Sat, 23 May 2026 18:16:39 -0700 Subject: [PATCH] policy: fix port range splitting crash due to u16 overflow This commit fixes a crash in proxy during port range splitting caused by u16 overflow when inserting a port range with `end_port` U16_MAX that overlaps with existing port range end port. The crash happens due to port range splitting algorithm incorrectly attempting to insert a overlapping port range that should already be covered by an existing port range. This commit fixes the issue by skipping port range insert when the new range is already covered by iterator at the end. Fixes cilium/cilium#45811 Signed-off-by: Deepesh Pathak --- cilium/network_policy.cc | 14 +++++--- tests/cilium_network_policy_test.cc | 56 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index dfea381aa..dddfcdaac 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -1597,7 +1597,7 @@ class PortNetworkPolicy : public Logger::Loggable { // 'start_key' is updated as needed below. auto start_range = it->first; - // split the current entry due to partial overlap in the beginning? + // split the current entry due to partial overlap in the beginning. // For example, if the current entry is 80-8080 and we are adding 4040-9999, // the current entry should be split to two ranges 80-4039 and 4040-8080, // both of which should retain their current rules, but new rules should only be @@ -1620,6 +1620,7 @@ class PortNetworkPolicy : public Logger::Loggable { } // scan the range of the new rule, filling the gaps with new (partial) ranges + bool new_range_end_covered = false; for (; it != rules_.end() && port <= end_port && end_port >= it->first.first; it++) { auto range = it->first; // create a new entry below the current one? @@ -1644,7 +1645,7 @@ class PortNetworkPolicy : public Logger::Loggable { port = range.first; } RELEASE_ASSERT(port == range.first, "port should match the start of the current range"); - // split the current range into two due to partial overlap in the end? + // split the current range into two due to partial overlap in the end. if (end_port < range.second) { auto rules = it->second; PortRange range1 = it->first; @@ -1661,12 +1662,17 @@ class PortNetworkPolicy : public Logger::Loggable { port = end_port + 1; // one past the end break; } else { + // Current range iterator completely covers the range to insert at the end. + if (range.second == end_port) { + new_range_end_covered = true; + break; + } // current entry completely covered by the new range, skip to the next port = range.second + 1; } } - // create a new entry covering the end? - if (port <= end_port) { + // create a new entry covering the end(if not already covered). + if (!new_range_end_covered && port <= end_port) { auto new_range = std::make_pair(port, end_port); auto new_pair = rules_.emplace(new_range, PortNetworkPolicyRules{}); RELEASE_ASSERT(new_pair.second, diff --git a/tests/cilium_network_policy_test.cc b/tests/cilium_network_policy_test.cc index b74055409..2e4d42d22 100644 --- a/tests/cilium_network_policy_test.cc +++ b/tests/cilium_network_policy_test.cc @@ -511,6 +511,62 @@ TEST_F(CiliumNetworkPolicyTest, OverlappingPortRanges) { // No egress is allowed: EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 8080)); EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 8080)); + + // Validate no overflow when splitting port ranges with existing port range containing U16 max. + EXPECT_NO_THROW(updateFromYaml(R"EOF(version_info: "1" +resources: +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: + - "10.1.2.3" + endpoint_id: 42 + ingress_per_port_policies: + - port: 32768 + end_port: 65534 + rules: + - remote_policies: [ 43 ] + - port: 32768 + end_port: 65535 + rules: + - remote_policies: [ 44 ] + - port: 1024 + end_port: 65535 + rules: + - remote_policies: [ 45 ] +)EOF")); + + expected = R"EOF(ingress: + rules: + [1024-32767]: + - rules: + - remotes: [45] + [32768-65534]: + - rules: + - remotes: [43] + - remotes: [44] + - remotes: [45] + [65535-65535]: + - rules: + - remotes: [44] + - remotes: [45] +egress: + rules: [] +)EOF"; + + EXPECT_TRUE(validate("10.1.2.3", expected)); + + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 40000)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 65535)); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 44, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 40000)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 44, 65535)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 45, 8080)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 45, 40000)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 45, 65535)); + + EXPECT_FALSE(egressAllowed("10.1.2.3", 43, 40000)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 44, 65535)); + EXPECT_FALSE(egressAllowed("10.1.2.3", 45, 8080)); } TEST_F(CiliumNetworkPolicyTest, DuplicatePorts) {