From 68a7648d14c1ed1b25ce17c7aea7ba4f5800858f Mon Sep 17 00:00:00 2001 From: Deniz Onur Duzgun <59659739+dduzgun-security@users.noreply.github.com> Date: Tue, 4 Jun 2024 17:55:53 -0400 Subject: [PATCH] security: resolve incorrect type conversions (#21251) * security: resolve incorrect type conversions * add changelog * fix more incorrect type conversions --- .changelog/21251.txt | 3 +++ agent/consul/leader_registrator_v2.go | 4 ++++ agent/xds/proxystateconverter/endpoints.go | 15 +++++++++------ agent/xds/proxystateconverter/listeners.go | 21 ++++++++++++--------- agent/xds/response/response.go | 17 ++++++++++------- agent/xds/testing.go | 6 +++--- api/api.go | 7 +++++++ 7 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 .changelog/21251.txt diff --git a/.changelog/21251.txt b/.changelog/21251.txt new file mode 100644 index 0000000000..ff4ef7cf23 --- /dev/null +++ b/.changelog/21251.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Fix multiple incorrect type conversion for potential overflows +``` diff --git a/agent/consul/leader_registrator_v2.go b/agent/consul/leader_registrator_v2.go index 97465e10d1..671fcc85d1 100644 --- a/agent/consul/leader_registrator_v2.go +++ b/agent/consul/leader_registrator_v2.go @@ -175,6 +175,10 @@ func (r V2ConsulRegistrator) createWorkloadFromMember(member serf.Member, parts workloadMeta["grpc_tls_port"] = strconv.Itoa(parts.ExternalGRPCTLSPort) } + if parts.Port < 0 || parts.Port > 65535 { + return nil, fmt.Errorf("invalid port: %d", parts.Port) + } + workload := &pbcatalog.Workload{ Addresses: []*pbcatalog.WorkloadAddress{ {Host: member.Addr.String(), Ports: []string{consulPortNameServer}}, diff --git a/agent/xds/proxystateconverter/endpoints.go b/agent/xds/proxystateconverter/endpoints.go index 2ed7164316..4e8f14d9cf 100644 --- a/agent/xds/proxystateconverter/endpoints.go +++ b/agent/xds/proxystateconverter/endpoints.go @@ -301,14 +301,17 @@ func (s *Converter) filterSubsetEndpoints(subset *structs.ServiceResolverSubset, // used in clusters.go func makeHostPortEndpoint(host string, port int) *pbproxystate.Endpoint { - return &pbproxystate.Endpoint{ - Address: &pbproxystate.Endpoint_HostPort{ - HostPort: &pbproxystate.HostPortAddress{ - Host: host, - Port: uint32(port), + if port >= 0 && port <= 65535 { + return &pbproxystate.Endpoint{ + Address: &pbproxystate.Endpoint_HostPort{ + HostPort: &pbproxystate.HostPortAddress{ + Host: host, + Port: uint32(port), + }, }, - }, + } } + return nil } func makeUnixSocketEndpoint(path string) *pbproxystate.Endpoint { diff --git a/agent/xds/proxystateconverter/listeners.go b/agent/xds/proxystateconverter/listeners.go index 11e966223b..7b4faeade5 100644 --- a/agent/xds/proxystateconverter/listeners.go +++ b/agent/xds/proxystateconverter/listeners.go @@ -764,17 +764,20 @@ func makeListenerWithDefault(opts makeListenerOpts) *pbproxystate.Listener { // // Since access logging is non-essential for routing, warn and move on // opts.logger.Warn("error generating access log xds", err) //} - return &pbproxystate.Listener{ - Name: fmt.Sprintf("%s:%s:%d", opts.name, opts.addr, opts.port), - //AccessLog: accessLog, - BindAddress: &pbproxystate.Listener_HostPort{ - HostPort: &pbproxystate.HostPortAddress{ - Host: opts.addr, - Port: uint32(opts.port), + if opts.port >= 0 && opts.port <= 65535 { + return &pbproxystate.Listener{ + Name: fmt.Sprintf("%s:%s:%d", opts.name, opts.addr, opts.port), + //AccessLog: accessLog, + BindAddress: &pbproxystate.Listener_HostPort{ + HostPort: &pbproxystate.HostPortAddress{ + Host: opts.addr, + Port: uint32(opts.port), + }, }, - }, - Direction: opts.direction, + Direction: opts.direction, + } } + return nil } func makePipeListener(opts makeListenerOpts) *pbproxystate.Listener { diff --git a/agent/xds/response/response.go b/agent/xds/response/response.go index cc6f132eb6..5b5d8de364 100644 --- a/agent/xds/response/response.go +++ b/agent/xds/response/response.go @@ -53,16 +53,19 @@ func MakePipeAddress(path string, mode uint32) *envoy_core_v3.Address { } func MakeAddress(ip string, port int) *envoy_core_v3.Address { - return &envoy_core_v3.Address{ - Address: &envoy_core_v3.Address_SocketAddress{ - SocketAddress: &envoy_core_v3.SocketAddress{ - Address: ip, - PortSpecifier: &envoy_core_v3.SocketAddress_PortValue{ - PortValue: uint32(port), + if port >= 0 && port <= 65535 { + return &envoy_core_v3.Address{ + Address: &envoy_core_v3.Address_SocketAddress{ + SocketAddress: &envoy_core_v3.SocketAddress{ + Address: ip, + PortSpecifier: &envoy_core_v3.SocketAddress_PortValue{ + PortValue: uint32(port), + }, }, }, - }, + } } + return nil } func MakeUint32Value(n int) *wrapperspb.UInt32Value { diff --git a/agent/xds/testing.go b/agent/xds/testing.go index 916bbd9b50..6d90005cd7 100644 --- a/agent/xds/testing.go +++ b/agent/xds/testing.go @@ -125,15 +125,15 @@ func stringToEnvoyVersion(vs string) (*envoy_type_v3.SemanticVersion, bool) { return nil, false } - major, err := strconv.Atoi(parts[0]) + major, err := strconv.ParseUint(parts[0], 10, 32) if err != nil { return nil, false } - minor, err := strconv.Atoi(parts[1]) + minor, err := strconv.ParseUint(parts[1], 10, 32) if err != nil { return nil, false } - patch, err := strconv.Atoi(parts[2]) + patch, err := strconv.ParseUint(parts[2], 10, 32) if err != nil { return nil, false } diff --git a/api/api.go b/api/api.go index b90a45d92b..d4d853d5d4 100644 --- a/api/api.go +++ b/api/api.go @@ -10,6 +10,7 @@ import ( "encoding/json" "fmt" "io" + "math" "net" "net/http" "net/url" @@ -1181,6 +1182,9 @@ func parseQueryMeta(resp *http.Response, q *QueryMeta) error { if err != nil { return fmt.Errorf("Failed to parse X-Consul-LastContact: %v", err) } + if last > math.MaxInt64 { + return fmt.Errorf("X-Consul-LastContact Header value is out of range: %d", last) + } q.LastContact = time.Duration(last) * time.Millisecond // Parse the X-Consul-KnownLeader @@ -1222,6 +1226,9 @@ func parseQueryMeta(resp *http.Response, q *QueryMeta) error { if err != nil { return fmt.Errorf("Failed to parse Age Header: %v", err) } + if age > math.MaxInt64 { + return fmt.Errorf("Age Header value is out of range: %d", last) + } q.CacheAge = time.Duration(age) * time.Second }