diff --git a/api/envoy/api/v2/endpoint/endpoint_components.proto b/api/envoy/api/v2/endpoint/endpoint_components.proto index d7f209311697d..78d45e2e08d06 100644 --- a/api/envoy/api/v2/endpoint/endpoint_components.proto +++ b/api/envoy/api/v2/endpoint/endpoint_components.proto @@ -97,7 +97,8 @@ message LbEndpoint { // percentage of traffic for the endpoint. This percentage is then further // weighted by the endpoint's locality's load balancing weight from // LocalityLbEndpoints. If unspecified, each host is presumed to have equal - // weight in a locality. + // weight in a locality. The sum of the weights of all endpoints in the + // endpoint's locality must not exceed uint32_t maximal value (4294967295). google.protobuf.UInt32Value load_balancing_weight = 4 [(validate.rules).uint32 = {gte: 1}]; } @@ -116,7 +117,8 @@ message LocalityLbEndpoints { // Optional: Per priority/region/zone/sub_zone weight; at least 1. The load // balancing weight for a locality is divided by the sum of the weights of all // localities at the same priority level to produce the effective percentage - // of traffic for the locality. + // of traffic for the locality. The sum of the weights of all localities at + // the same priority level must not exceed uint32_t maximal value (4294967295). // // Locality weights are only considered when :ref:`locality weighted load // balancing ` is diff --git a/api/envoy/config/endpoint/v3/endpoint_components.proto b/api/envoy/config/endpoint/v3/endpoint_components.proto index ce7048b5baca7..b880a38d1a3ea 100644 --- a/api/envoy/config/endpoint/v3/endpoint_components.proto +++ b/api/envoy/config/endpoint/v3/endpoint_components.proto @@ -103,7 +103,8 @@ message LbEndpoint { // percentage of traffic for the endpoint. This percentage is then further // weighted by the endpoint's locality's load balancing weight from // LocalityLbEndpoints. If unspecified, each host is presumed to have equal - // weight in a locality. + // weight in a locality. The sum of the weights of all endpoints in the + // endpoint's locality must not exceed uint32_t maximal value (4294967295). google.protobuf.UInt32Value load_balancing_weight = 4 [(validate.rules).uint32 = {gte: 1}]; } @@ -125,7 +126,8 @@ message LocalityLbEndpoints { // Optional: Per priority/region/zone/sub_zone weight; at least 1. The load // balancing weight for a locality is divided by the sum of the weights of all // localities at the same priority level to produce the effective percentage - // of traffic for the locality. + // of traffic for the locality. The sum of the weights of all localities at + // the same priority level must not exceed uint32_t maximal value (4294967295). // // Locality weights are only considered when :ref:`locality weighted load // balancing ` is diff --git a/generated_api_shadow/envoy/api/v2/endpoint/endpoint_components.proto b/generated_api_shadow/envoy/api/v2/endpoint/endpoint_components.proto index d7f209311697d..78d45e2e08d06 100644 --- a/generated_api_shadow/envoy/api/v2/endpoint/endpoint_components.proto +++ b/generated_api_shadow/envoy/api/v2/endpoint/endpoint_components.proto @@ -97,7 +97,8 @@ message LbEndpoint { // percentage of traffic for the endpoint. This percentage is then further // weighted by the endpoint's locality's load balancing weight from // LocalityLbEndpoints. If unspecified, each host is presumed to have equal - // weight in a locality. + // weight in a locality. The sum of the weights of all endpoints in the + // endpoint's locality must not exceed uint32_t maximal value (4294967295). google.protobuf.UInt32Value load_balancing_weight = 4 [(validate.rules).uint32 = {gte: 1}]; } @@ -116,7 +117,8 @@ message LocalityLbEndpoints { // Optional: Per priority/region/zone/sub_zone weight; at least 1. The load // balancing weight for a locality is divided by the sum of the weights of all // localities at the same priority level to produce the effective percentage - // of traffic for the locality. + // of traffic for the locality. The sum of the weights of all localities at + // the same priority level must not exceed uint32_t maximal value (4294967295). // // Locality weights are only considered when :ref:`locality weighted load // balancing ` is diff --git a/generated_api_shadow/envoy/config/endpoint/v3/endpoint_components.proto b/generated_api_shadow/envoy/config/endpoint/v3/endpoint_components.proto index 60df915f2a9fb..8e800745df3f6 100644 --- a/generated_api_shadow/envoy/config/endpoint/v3/endpoint_components.proto +++ b/generated_api_shadow/envoy/config/endpoint/v3/endpoint_components.proto @@ -102,7 +102,8 @@ message LbEndpoint { // percentage of traffic for the endpoint. This percentage is then further // weighted by the endpoint's locality's load balancing weight from // LocalityLbEndpoints. If unspecified, each host is presumed to have equal - // weight in a locality. + // weight in a locality. The sum of the weights of all endpoints in the + // endpoint's locality must not exceed uint32_t maximal value (4294967295). string endpoint_name = 5; } } @@ -125,7 +126,8 @@ message LocalityLbEndpoints { // Optional: Per priority/region/zone/sub_zone weight; at least 1. The load // balancing weight for a locality is divided by the sum of the weights of all // localities at the same priority level to produce the effective percentage - // of traffic for the locality. + // of traffic for the locality. The sum of the weights of all localities at + // the same priority level must not exceed uint32_t maximal value (4294967295). // // Locality weights are only considered when :ref:`locality weighted load // balancing ` is diff --git a/source/common/upstream/thread_aware_lb_impl.cc b/source/common/upstream/thread_aware_lb_impl.cc index 5ede9a31b64f8..5e047de8415f8 100644 --- a/source/common/upstream/thread_aware_lb_impl.cc +++ b/source/common/upstream/thread_aware_lb_impl.cc @@ -12,9 +12,16 @@ namespace { void normalizeHostWeights(const HostVector& hosts, double normalized_locality_weight, NormalizedHostWeightVector& normalized_host_weights, double& min_normalized_weight, double& max_normalized_weight) { - uint32_t sum = 0; + // sum should be at most uint32_t max value, so we can validate it by accumulating into unit64_t + // and making sure there was no overflow + uint64_t sum = 0; for (const auto& host : hosts) { sum += host->weight(); + if (sum > std::numeric_limits::max()) { + throw EnvoyException( + fmt::format("The sum of weights of all upstream hosts in a locality exceeds {}", + std::numeric_limits::max())); + } } for (const auto& host : hosts) { @@ -31,9 +38,16 @@ void normalizeLocalityWeights(const HostsPerLocality& hosts_per_locality, double& min_normalized_weight, double& max_normalized_weight) { ASSERT(locality_weights.size() == hosts_per_locality.get().size()); - uint32_t sum = 0; + // sum should be at most uint32_t max value, so we can validate it by accumulating into unit64_t + // and making sure there was no overflow + uint64_t sum = 0; for (const auto weight : locality_weights) { sum += weight; + if (sum > std::numeric_limits::max()) { + throw EnvoyException( + fmt::format("The sum of weights of all localities at the same priority exceeds {}", + std::numeric_limits::max())); + } } // Locality weights (unlike host weights) may be 0. If _all_ locality weights were 0, bail out. diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index d8076d337cd84..6806d8d2a639e 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -545,6 +545,191 @@ TEST_F(ConfigurationImplTest, AdminSocketOptions) { EXPECT_EQ(detail->name_, Envoy::Network::SocketOptionName(4, 5, "4/5")); } +TEST_F(ConfigurationImplTest, ExceedLoadBalancerHostWeightsLimit) { + const std::string json = R"EOF( + { + "static_resources": { + "listeners" : [], + "clusters": [ + { + "name": "test_cluster", + "type": "static", + "connect_timeout": "0.01s", + "per_connection_buffer_limit_bytes": 8192, + "lb_policy": "RING_HASH", + "load_assignment": { + "cluster_name": "load_test_cluster", + "endpoints": [ + { + "priority": 93 + }, + { + "locality": { + "zone": "zone1" + }, + "lb_endpoints": [ + { + "endpoint": { + "address": { + "pipe": { + "path": "path/to/pipe" + } + } + }, + "health_status": "TIMEOUT", + "load_balancing_weight": { + "value": 4294967295 + } + }, + { + "endpoint": { + "address": { + "pipe": { + "path": "path/to/pipe2" + } + } + }, + "health_status": "TIMEOUT", + "load_balancing_weight": { + "value": 1 + } + } + ], + "load_balancing_weight": { + "value": 122 + } + } + ] + } + } + ] + }, + "admin": { + "access_log_path": "/dev/null", + "address": { + "socket_address": { + "address": "1.2.3.4", + "port_value": 5678 + } + } + } + } + )EOF"; + + auto bootstrap = Upstream::parseBootstrapFromV2Json(json); + + MainImpl config; + EXPECT_THROW_WITH_MESSAGE( + config.initialize(bootstrap, server_, cluster_manager_factory_), EnvoyException, + "The sum of weights of all upstream hosts in a locality exceeds 4294967295"); +} + +TEST_F(ConfigurationImplTest, ExceedLoadBalancerLocalityWeightsLimit) { + const std::string json = R"EOF( + { + "static_resources": { + "listeners" : [], + "clusters": [ + { + "name": "test_cluster", + "type": "static", + "connect_timeout": "0.01s", + "per_connection_buffer_limit_bytes": 8192, + "lb_policy": "RING_HASH", + "load_assignment": { + "cluster_name": "load_test_cluster", + "endpoints": [ + { + "priority": 93 + }, + { + "locality": { + "zone": "zone1" + }, + "lb_endpoints": [ + { + "endpoint": { + "address": { + "pipe": { + "path": "path/to/pipe" + } + } + }, + "health_status": "TIMEOUT", + "load_balancing_weight": { + "value": 7 + } + } + ], + "load_balancing_weight": { + "value": 4294967295 + } + }, + { + "locality": { + "region": "domains", + "sub_zone": "sub_zone1" + }, + "lb_endpoints": [ + { + "endpoint": { + "address": { + "pipe": { + "path": "path/to/pipe" + } + } + }, + "health_status": "TIMEOUT", + "load_balancing_weight": { + "value": 8 + } + } + ], + "load_balancing_weight": { + "value": 2 + } + } + ] + }, + "lb_subset_config": { + "fallback_policy": "ANY_ENDPOINT", + "subset_selectors": { + "keys": [ + "x" + ] + }, + "locality_weight_aware": "true" + }, + "common_lb_config": { + "healthy_panic_threshold": { + "value": 0.8 + }, + "locality_weighted_lb_config": { + } + } + } + ] + }, + "admin": { + "access_log_path": "/dev/null", + "address": { + "socket_address": { + "address": "1.2.3.4", + "port_value": 5678 + } + } + } + } + )EOF"; + + auto bootstrap = Upstream::parseBootstrapFromV2Json(json); + + MainImpl config; + EXPECT_THROW_WITH_MESSAGE( + config.initialize(bootstrap, server_, cluster_manager_factory_), EnvoyException, + "The sum of weights of all localities at the same priority exceeds 4294967295"); +} + } // namespace } // namespace Configuration } // namespace Server diff --git a/test/server/server_corpus/clusterfuzz-testcase-minimized-config_fuzz_test-5702999713513472 b/test/server/server_corpus/clusterfuzz-testcase-minimized-config_fuzz_test-5702999713513472 new file mode 100644 index 0000000000000..1aed2296c4bc0 --- /dev/null +++ b/test/server/server_corpus/clusterfuzz-testcase-minimized-config_fuzz_test-5702999713513472 @@ -0,0 +1,249 @@ +static_resources { + listeners { + address { + pipe { + path: "@" + } + } + } + clusters { + name: "&" + connect_timeout { + seconds: 2304 + } + lb_policy: RING_HASH + alt_stat_name: "t" + close_connections_on_host_health_failure: true + track_timeout_budgets: true + } + clusters { + name: "servi|e_gogole" + connect_timeout { + seconds: 3 + } + lb_policy: RING_HASH + load_assignment { + cluster_name: "domains" + endpoints { + priority: 93 + } + endpoints { + locality { + zone: "t" + } + lb_endpoints { + endpoint { + address { + pipe { + path: ";" + } + } + } + health_status: TIMEOUT + load_balancing_weight { + value: 4294967295 + } + } + load_balancing_weight { + value: 122 + } + } + endpoints { + locality { + region: "domains" + sub_zone: "|" + } + lb_endpoints { + endpoint { + address { + pipe { + path: ";" + } + } + } + health_status: TIMEOUT + } + priority: 122 + } + endpoints { + locality { + zone: "\t" + } + load_balancing_weight { + value: 122 + } + priority: 122 + } + endpoints { + locality { + sub_zone: "|" + } + priority: 122 + } + endpoints { + locality { + zone: "\t" + } + load_balancing_weight { + value: 285212672 + } + priority: 122 + } + endpoints { + locality { + } + load_balancing_weight { + value: 122 + } + priority: 122 + } + endpoints { + locality { + zone: "\t" + } + lb_endpoints { + endpoint { + address { + socket_address { + address: "0.0.0.0" + port_value: 47 + } + } + } + health_status: TIMEOUT + } + load_balancing_weight { + value: 122 + } + } + endpoints { + locality { + zone: "\t" + } + lb_endpoints { + endpoint { + address { + pipe { + path: "@" + } + } + } + health_status: UNHEALTHY + } + load_balancing_weight { + value: 122 + } + priority: 122 + } + endpoints { + load_balancing_weight { + value: 122 + } + priority: 122 + } + policy { + } + } + } +} +flags_path: "VVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVVV(VVVVVVVVVVVVVVV" +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { + typed_config { + type_url: "type.googleapis.com/e" + } +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { + typed_config { + type_url: "type.googleapis.com/envoy.api.v2.route.Route" + value: "B\004\n\000\022\000R\000" + } +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { + hidden_envoy_deprecated_config { + fields { + key: "" + value { + null_value: NULL_VALUE + } + } + } +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { + name: "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000" +} +stats_sinks { + typed_config { + type_url: "\000\000\000\000" + } +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { + typed_config { + type_url: "type.googleapis.com/e" + } +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { +} +stats_sinks { + name: "ffff]f$fffff" +} +stats_sinks { + name: "\000\035eut_c" +} +stats_sinks { + name: "]" +} +admin { + profile_path: "\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'\'" +}