Skip to content

Commit f158ba8

Browse files
amuraruamuraru
committed
Fix "Incorrect conversion between integer types" reported by CodeQL (#142)
1 parent 60a8a3a commit f158ba8

File tree

9 files changed

+156
-17
lines changed

9 files changed

+156
-17
lines changed

‎pkg/kafkaclient/topics.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package kafkaclient
1717
import (
1818
"errors"
1919
"fmt"
20+
"math"
2021
"time"
2122

2223
"github.com/IBM/sarama"
@@ -122,7 +123,13 @@ func (k *kafkaClient) EnsurePartitionCount(topic string, desired int32) (changed
122123
return
123124
}
124125

125-
if desired != int32(len(meta[0].Partitions)) {
126+
currentPartitions := len(meta[0].Partitions)
127+
// Partition count should always be within valid range for int32 conversion
128+
if currentPartitions < 0 || currentPartitions > math.MaxInt32 {
129+
err = errorfactory.New(errorfactory.BrokersRequestError{}, fmt.Errorf("partition count %d out of valid range for int32 conversion", currentPartitions), "invalid partition count")
130+
return
131+
}
132+
if desired != int32(currentPartitions) {
126133
// TODO (tinyzimmer): maybe let the user specify partition assignments
127134
assn := make([][]int32, 0)
128135
changed = true

‎pkg/resources/envoy/configmap.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package envoy
1616

1717
import (
1818
"fmt"
19+
"math"
1920
"sort"
2021

2122
envoyaccesslog "github.com/envoyproxy/go-control-plane/envoy/config/accesslog/v3"
@@ -339,7 +340,15 @@ func GenerateEnvoyConfig(kc *v1beta1.KafkaCluster, elistener v1beta1.ExternalLis
339340
}
340341

341342
if elistener.TLSEnabled() {
342-
filterChain, err = GenerateEnvoyTLSFilterChain(tcpProxy, ingressConfig.EnvoyConfig.GetBrokerHostname(int32(brokerId)), log)
343+
filterChain, err = GenerateEnvoyTLSFilterChain(tcpProxy, func() string {
344+
// Broker IDs are always within valid range for int32 conversion
345+
if brokerId < 0 || brokerId > math.MaxInt32 {
346+
// This should never happen as broker IDs are small positive integers
347+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in envoy TLS filter chain")
348+
return ""
349+
}
350+
return ingressConfig.EnvoyConfig.GetBrokerHostname(int32(brokerId))
351+
}(), log)
343352
if err != nil {
344353
log.Error(err, "Unable to generate broker envoy tls filter chain")
345354
return ""
@@ -352,7 +361,15 @@ func GenerateEnvoyConfig(kc *v1beta1.KafkaCluster, elistener v1beta1.ExternalLis
352361
}
353362
}
354363

355-
brokerPort := elistener.GetBrokerPort(int32(brokerId))
364+
brokerPort := func() int32 {
365+
// Broker IDs are always within valid range for int32 conversion
366+
if brokerId < 0 || brokerId > math.MaxInt32 {
367+
// This should never happen as broker IDs are small positive integers
368+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in envoy broker port assignment")
369+
return 0
370+
}
371+
return elistener.GetBrokerPort(int32(brokerId))
372+
}()
356373
tempListeners[brokerPort] = append(tempListeners[brokerPort], filterChain)
357374

358375
clusters = append(clusters, &envoycluster.Cluster{

‎pkg/resources/envoy/deployment.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"crypto/sha256"
1919
"encoding/hex"
2020
"fmt"
21+
"math"
2122
"strconv"
2223

2324
"github.com/go-logr/logr"
@@ -157,9 +158,17 @@ func getExposedContainerPorts(extListener v1beta1.ExternalListenerConfig, broker
157158
}
158159
if util.ShouldIncludeBroker(brokerConfig, kafkaCluster.Status, brokerId, defaultIngressConfigName, ingressConfigName) && !extListener.TLSEnabled() {
159160
exposedPorts = append(exposedPorts, corev1.ContainerPort{
160-
Name: fmt.Sprintf("broker-%d", brokerId),
161-
ContainerPort: extListener.GetBrokerPort(int32(brokerId)),
162-
Protocol: corev1.ProtocolTCP,
161+
Name: fmt.Sprintf("broker-%d", brokerId),
162+
ContainerPort: func() int32 {
163+
// Broker IDs are always within valid range for int32 conversion
164+
if brokerId < 0 || brokerId > math.MaxInt32 {
165+
// This should never happen as broker IDs are small positive integers
166+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in envoy deployment container port")
167+
return 0
168+
}
169+
return extListener.GetBrokerPort(int32(brokerId))
170+
}(),
171+
Protocol: corev1.ProtocolTCP,
163172
})
164173
}
165174
}

‎pkg/resources/envoy/service.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package envoy
1616

1717
import (
1818
"fmt"
19+
"math"
1920

2021
"github.com/go-logr/logr"
2122
"k8s.io/apimachinery/pkg/runtime"
@@ -72,10 +73,33 @@ func getExposedServicePorts(extListener v1beta1.ExternalListenerConfig, brokersI
7273
}
7374
if util.ShouldIncludeBroker(brokerConfig, kafkaCluster.Status, brokerId, defaultIngressConfigName, ingressConfigName) {
7475
exposedPorts = append(exposedPorts, corev1.ServicePort{
75-
Name: fmt.Sprintf("broker-%d", brokerId),
76-
Port: extListener.GetBrokerPort(int32(brokerId)),
77-
TargetPort: intstr.FromInt(int(extListener.GetBrokerPort(int32(brokerId)))),
78-
Protocol: corev1.ProtocolTCP,
76+
Name: fmt.Sprintf("broker-%d", brokerId),
77+
Port: func() int32 {
78+
// Broker IDs are always within valid range for int32 conversion
79+
if brokerId < 0 || brokerId > math.MaxInt32 {
80+
// This should never happen as broker IDs are small positive integers
81+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in envoy service port")
82+
return 0
83+
}
84+
return extListener.GetBrokerPort(int32(brokerId))
85+
}(),
86+
TargetPort: func() intstr.IntOrString {
87+
// Broker IDs are always within valid range for int32 conversion
88+
if brokerId < 0 || brokerId > math.MaxInt32 {
89+
// This should never happen as broker IDs are small positive integers
90+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in envoy service target port")
91+
return intstr.FromInt(0)
92+
}
93+
brokerPort := extListener.GetBrokerPort(int32(brokerId))
94+
// Port numbers are always within valid range for int conversion
95+
if brokerPort < 0 || brokerPort > 65535 {
96+
// This should never happen as GetBrokerPort returns valid port numbers
97+
log.Error(fmt.Errorf("broker port %d out of valid range [0-65535] for broker %d", brokerPort, brokerId), "Invalid broker port detected in envoy service target port")
98+
return intstr.FromInt(0)
99+
}
100+
return intstr.FromInt(int(brokerPort))
101+
}(),
102+
Protocol: corev1.ProtocolTCP,
79103
})
80104
}
81105
}

‎pkg/resources/istioingress/gateway.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package istioingress
1616

1717
import (
1818
"fmt"
19+
"math"
1920

2021
istioclientv1beta1 "github.com/banzaicloud/istio-client-go/pkg/networking/v1beta1"
2122

@@ -70,7 +71,22 @@ func generateServers(kc *v1beta1.KafkaCluster, externalListenerConfig v1beta1.Ex
7071
if util.ShouldIncludeBroker(brokerConfig, kc.Status, brokerId, defaultIngressConfigName, ingressConfigName) {
7172
servers = append(servers, istioclientv1beta1.Server{
7273
Port: &istioclientv1beta1.Port{
73-
Number: int(externalListenerConfig.GetBrokerPort(int32(brokerId))),
74+
Number: func() int {
75+
// Broker IDs are always within valid range for int32 conversion
76+
if brokerId < 0 || brokerId > math.MaxInt32 {
77+
// This should never happen as broker IDs are small positive integers
78+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in gateway port")
79+
return 0
80+
}
81+
brokerPort := externalListenerConfig.GetBrokerPort(int32(brokerId))
82+
// Port numbers are always within valid range for int conversion
83+
if brokerPort < 0 || brokerPort > 65535 {
84+
// This should never happen as GetBrokerPort returns valid port numbers
85+
log.Error(fmt.Errorf("broker port %d out of valid range [0-65535] for broker %d", brokerPort, brokerId), "Invalid broker port detected in gateway port")
86+
return 0
87+
}
88+
return int(brokerPort)
89+
}(),
7490
Protocol: protocol,
7591
Name: fmt.Sprintf("tcp-broker-%d", brokerId),
7692
},

‎pkg/resources/istioingress/meshgateway.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package istioingress
1616

1717
import (
1818
"fmt"
19+
"math"
1920

2021
istioOperatorApi "github.com/banzaicloud/istio-operator/api/v2/v1alpha1"
2122
"github.com/go-logr/logr"
@@ -99,10 +100,33 @@ func generateExternalPorts(kc *v1beta1.KafkaCluster, brokerIds []int,
99100
}
100101
if util.ShouldIncludeBroker(brokerConfig, kc.Status, brokerId, defaultIngressConfigName, ingressConfigName) {
101102
generatedPorts = append(generatedPorts, &istioOperatorApi.ServicePort{
102-
Name: fmt.Sprintf("tcp-broker-%d", brokerId),
103-
Protocol: string(corev1.ProtocolTCP),
104-
Port: externalListenerConfig.GetBrokerPort(int32(brokerId)),
105-
TargetPort: &istioOperatorApi.IntOrString{IntOrString: intstr.FromInt(int(externalListenerConfig.GetBrokerPort(int32(brokerId))))},
103+
Name: fmt.Sprintf("tcp-broker-%d", brokerId),
104+
Protocol: string(corev1.ProtocolTCP),
105+
Port: func() int32 {
106+
// Broker IDs are always within valid range for int32 conversion
107+
if brokerId < 0 || brokerId > math.MaxInt32 {
108+
// This should never happen as broker IDs are small positive integers
109+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in mesh gateway port")
110+
return 0
111+
}
112+
return externalListenerConfig.GetBrokerPort(int32(brokerId))
113+
}(),
114+
TargetPort: func() *istioOperatorApi.IntOrString {
115+
// Broker IDs are always within valid range for int32 conversion
116+
if brokerId < 0 || brokerId > math.MaxInt32 {
117+
// This should never happen as broker IDs are small positive integers
118+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in mesh gateway target port")
119+
return &istioOperatorApi.IntOrString{IntOrString: intstr.FromInt(0)}
120+
}
121+
brokerPort := externalListenerConfig.GetBrokerPort(int32(brokerId))
122+
// Port numbers are always within valid range for int conversion
123+
if brokerPort < 0 || brokerPort > 65535 {
124+
// This should never happen as GetBrokerPort returns valid port numbers
125+
log.Error(fmt.Errorf("broker port %d out of valid range [0-65535] for broker %d", brokerPort, brokerId), "Invalid broker port detected in mesh gateway target port")
126+
return &istioOperatorApi.IntOrString{IntOrString: intstr.FromInt(0)}
127+
}
128+
return &istioOperatorApi.IntOrString{IntOrString: intstr.FromInt(int(brokerPort))}
129+
}(),
106130
})
107131
}
108132
}

‎pkg/resources/istioingress/virtualservice.go‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package istioingress
1616

1717
import (
1818
"fmt"
19+
"math"
1920

2021
istioclientv1beta1 "github.com/banzaicloud/istio-client-go/pkg/networking/v1beta1"
2122

@@ -79,7 +80,22 @@ func generateTlsRoutes(kc *v1beta1.KafkaCluster, externalListenerConfig v1beta1.
7980
tlsRoutes = append(tlsRoutes, istioclientv1beta1.TLSRoute{
8081
Match: []istioclientv1beta1.TLSMatchAttributes{
8182
{
82-
Port: util.IntPointer(int(externalListenerConfig.GetBrokerPort(int32(brokerId)))),
83+
Port: func() *int {
84+
// Broker IDs are always within valid range for int32 conversion
85+
if brokerId < 0 || brokerId > math.MaxInt32 {
86+
// This should never happen as broker IDs are small positive integers
87+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in TLS route port")
88+
return util.IntPointer(0)
89+
}
90+
brokerPort := externalListenerConfig.GetBrokerPort(int32(brokerId))
91+
// Port numbers are always within valid range for int conversion
92+
if brokerPort < 0 || brokerPort > 65535 {
93+
// This should never happen as GetBrokerPort returns valid port numbers
94+
log.Error(fmt.Errorf("broker port %d out of valid range [0-65535] for broker %d", brokerPort, brokerId), "Invalid broker port detected in TLS route")
95+
return util.IntPointer(0)
96+
}
97+
return util.IntPointer(int(brokerPort))
98+
}(),
8399
SniHosts: []string{"*"},
84100
},
85101
},
@@ -132,7 +148,22 @@ func generateTcpRoutes(kc *v1beta1.KafkaCluster, externalListenerConfig v1beta1.
132148
tcpRoutes = append(tcpRoutes, istioclientv1beta1.TCPRoute{
133149
Match: []istioclientv1beta1.L4MatchAttributes{
134150
{
135-
Port: util.IntPointer(int(externalListenerConfig.GetBrokerPort(int32(brokerId)))),
151+
Port: func() *int {
152+
// Broker IDs are always within valid range for int32 conversion
153+
if brokerId < 0 || brokerId > math.MaxInt32 {
154+
// This should never happen as broker IDs are small positive integers
155+
log.Error(fmt.Errorf("broker ID %d out of valid range for int32 conversion", brokerId), "Invalid broker ID detected in TCP route port")
156+
return util.IntPointer(0)
157+
}
158+
brokerPort := externalListenerConfig.GetBrokerPort(int32(brokerId))
159+
// Port numbers are always within valid range for int conversion
160+
if brokerPort < 0 || brokerPort > 65535 {
161+
// This should never happen as GetBrokerPort returns valid port numbers
162+
log.Error(fmt.Errorf("broker port %d out of valid range [0-65535] for broker %d", brokerPort, brokerId), "Invalid broker port detected in TCP route")
163+
return util.IntPointer(0)
164+
}
165+
return util.IntPointer(int(brokerPort))
166+
}(),
136167
},
137168
},
138169
Route: []*istioclientv1beta1.RouteDestination{

‎pkg/scale/scale.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,12 @@ func (cc *cruiseControlScaler) RemoveBrokers(ctx context.Context, brokerIDs ...s
509509
cc.log.Error(err, "failed to cast broker ID from string to integer", "broker_id", brokerID)
510510
return nil, err
511511
}
512+
// Broker IDs are always within valid range for int32 conversion
513+
if bID < 0 || bID > math.MaxInt32 {
514+
err := fmt.Errorf("broker ID %d out of valid range for int32 conversion", bID)
515+
cc.log.Error(err, "invalid broker ID detected", "broker_id", brokerID)
516+
return nil, err
517+
}
512518
brokersToRemove = append(brokersToRemove, int32(bID))
513519
}
514520

‎pkg/scale/utils.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package scale
1616

1717
import (
1818
"fmt"
19+
"math"
1920
"strconv"
2021

2122
"github.com/banzaicloud/koperator/api/v1beta1"
@@ -28,6 +29,10 @@ func brokerIDsFromStringSlice(brokerIDs []string) ([]int32, error) {
2829
if err != nil {
2930
return nil, err
3031
}
32+
// Broker IDs are always within valid range for int32 conversion
33+
if bid < 0 || bid > math.MaxInt32 {
34+
return nil, fmt.Errorf("broker ID %d out of valid range for int32 conversion", bid)
35+
}
3136
brokers[idx] = int32(bid)
3237
}
3338
return brokers, nil

0 commit comments

Comments
 (0)