From d2ff77886ef4c084d5417be1d8ea044da38bda61 Mon Sep 17 00:00:00 2001 From: Dominic Evans Date: Wed, 28 Apr 2021 10:52:23 +0100 Subject: [PATCH] fix: correct initial CodeQL findings - Incorrect conversion between integer types - Size computation for allocation may overflow --- admin.go | 12 ++++++++++-- broker.go | 2 +- gssapi_kerberos.go | 22 +++++++++------------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/admin.go b/admin.go index e0b102034..4aa3cf1d5 100644 --- a/admin.go +++ b/admin.go @@ -619,7 +619,11 @@ func (ca *clusterAdmin) DescribeConfig(resource ConfigResource) ([]ConfigEntry, // DescribeConfig of broker/broker logger must be sent to the broker in question if dependsOnSpecificNode(resource) { - id, _ := strconv.Atoi(resource.Name) + var id int64 + id, err = strconv.ParseInt(resource.Name, 10, 32) + if err != nil { + return nil, err + } b, err = ca.findBroker(int32(id)) } else { b, err = ca.findAnyBroker() @@ -670,7 +674,11 @@ func (ca *clusterAdmin) AlterConfig(resourceType ConfigResourceType, name string // AlterConfig of broker/broker logger must be sent to the broker in question if dependsOnSpecificNode(ConfigResource{Name: name, Type: resourceType}) { - id, _ := strconv.Atoi(name) + var id int64 + id, err = strconv.ParseInt(name, 10, 32) + if err != nil { + return err + } b, err = ca.findBroker(int32(id)) } else { b, err = ca.findAnyBroker() diff --git a/broker.go b/broker.go index 5858a23c0..a466689cd 100644 --- a/broker.go +++ b/broker.go @@ -816,7 +816,7 @@ func (b *Broker) encode(pe packetEncoder, version int16) (err error) { return err } - port, err := strconv.Atoi(portstr) + port, err := strconv.ParseInt(portstr, 10, 32) if err != nil { return err } diff --git a/gssapi_kerberos.go b/gssapi_kerberos.go index 44fd44625..c5a6113a8 100644 --- a/gssapi_kerberos.go +++ b/gssapi_kerberos.go @@ -3,8 +3,10 @@ package sarama import ( "encoding/asn1" "encoding/binary" + "errors" "fmt" "io" + "math" "strings" "time" @@ -53,15 +55,14 @@ type KerberosClient interface { Destroy() } -/* -* -* Appends length in big endian before payload, and send it to kafka -* - */ - +// writePackage appends length in big endian before the payload, and sends it to kafka func (krbAuth *GSSAPIKerberosAuth) writePackage(broker *Broker, payload []byte) (int, error) { length := len(payload) - finalPackage := make([]byte, length+4) //4 byte length header + payload + size := length + 4 // 4 byte length header + payload + if size > math.MaxUint32 { + return 0, errors.New("payload too large, will overflow uint32") + } + finalPackage := make([]byte, size) copy(finalPackage[4:], payload) binary.BigEndian.PutUint32(finalPackage, uint32(length)) bytes, err := broker.conn.Write(finalPackage) @@ -71,12 +72,7 @@ func (krbAuth *GSSAPIKerberosAuth) writePackage(broker *Broker, payload []byte) return bytes, nil } -/* -* -* Read length (4 bytes) and then read the payload -* - */ - +// readPackage reads payload length (4 bytes) and then reads the payload into []byte func (krbAuth *GSSAPIKerberosAuth) readPackage(broker *Broker) ([]byte, int, error) { bytesRead := 0 lengthInBytes := make([]byte, 4)