Fix port range splitting crash due to u16 overflow

cilium/network_policy.cc

@@ -1597,7 +1597,7 @@ class PortNetworkPolicy : public Logger::Loggable<Logger::Id::config> {
           // '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<Logger::Id::config> {
           }
 
           // 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<Logger::Id::config> {
               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<Logger::Id::config> {
               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,

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) {