gosec: handle integer conversions and potential overflows

Author: oxzi

cmd/icingadb-migrate/convert.go

@@ -11,6 +11,7 @@ import (
 	"github.com/icinga/icingadb/pkg/icingadb/v1/history"
 	"github.com/jmoiron/sqlx"
 	"github.com/pkg/errors"
+	"math"
 	"strconv"
 	"strings"
 	"time"
@@ -282,6 +283,14 @@ func convertDowntimeRows(
 			cancelTime = actualEnd
 		}
 
+		// The IDO duration is of type bigint, representing seconds, while Icinga DB's FlexibleDuration is an unsigned
+		// bigint, representing milliseconds. In theory, there should be no negative value in the IDO and multiplied
+		// with factor 1,000 should not overflow anything. In theory, at least. To make sure, invalid values are capped.
+		var flexibleDuration uint64
+		if durationSec := row.Duration; durationSec >= 0 && durationSec < math.MaxUint64/1_000 {
+			flexibleDuration = uint64(durationSec) * 1_000
+		}
+
 		downtimeHistory = append(downtimeHistory, &history.DowntimeHistory{
 			DowntimeHistoryEntity: history.DowntimeHistoryEntity{DowntimeId: id},
 			HistoryTableMeta: history.HistoryTableMeta{
@@ -299,7 +308,7 @@ func convertDowntimeRows(
 			Author:             row.AuthorName,
 			Comment:            row.CommentData,
 			IsFlexible:         types.Bool{Bool: row.IsFixed == 0, Valid: true},
-			FlexibleDuration:   uint64(row.Duration) * 1000,
+			FlexibleDuration:   flexibleDuration,
 			ScheduledStartTime: scheduledStart,
 			ScheduledEndTime:   scheduledEnd,
 			StartTime:          startTime,

pkg/icingadb/cleanup.go

@@ -32,7 +32,7 @@ func (stmt *CleanupStmt) CleanupOlderThan(
 	defer db.Log(ctx, q, &counter).Stop()
 
 	for {
-		var rowsDeleted int64
+		var rowsDeleted uint64
 
 		err := retry.WithBackoff(
 			ctx,
@@ -45,7 +45,10 @@ func (stmt *CleanupStmt) CleanupOlderThan(
 					return database.CantPerformQuery(err, q)
 				}
 
-				rowsDeleted, err = rs.RowsAffected()
+				i, err := rs.RowsAffected()
+				if err == nil && i >= 0 {
+					rowsDeleted = uint64(i)
+				}
 
 				return err
 			},
@@ -57,15 +60,15 @@ func (stmt *CleanupStmt) CleanupOlderThan(
 			return 0, err
 		}
 
-		counter.Add(uint64(rowsDeleted))
+		counter.Add(rowsDeleted)
 
 		for _, onSuccess := range onSuccess {
 			if err := onSuccess(ctx, make([]struct{}, rowsDeleted)); err != nil {
 				return 0, err
 			}
 		}
 
-		if rowsDeleted < int64(count) {
+		if rowsDeleted < count {
 			break
 		}
 	}

pkg/icingadb/delta_test.go

@@ -208,14 +208,14 @@ func testDeltaVerifyResult(t *testing.T, name string, expected map[uint64]uint64
 }
 
 func BenchmarkDelta(b *testing.B) {
-	for n := 1 << 10; n <= 1<<20; n <<= 1 {
-		b.Run(strconv.Itoa(n), func(b *testing.B) {
+	for n := uint64(1 << 10); n <= 1<<20; n <<= 1 {
+		b.Run(strconv.FormatUint(n, 10), func(b *testing.B) {
 			benchmarkDelta(b, n)
 		})
 	}
 }
 
-func benchmarkDelta(b *testing.B, numEntities int) {
+func benchmarkDelta(b *testing.B, numEntities uint64) {
 	chActual := make([]chan database.Entity, b.N)
 	chDesired := make([]chan database.Entity, b.N)
 	for i := 0; i < b.N; i++ {
@@ -231,20 +231,20 @@ func benchmarkDelta(b *testing.B, numEntities int) {
 		binary.BigEndian.PutUint64(e.PropertiesChecksum, checksum)
 		return e
 	}
-	for i := 0; i < numEntities; i++ {
+	for i := uint64(0); i < numEntities; i++ {
 		// each iteration writes exactly one entity to each channel
 		var eActual, eDesired database.Entity
 		switch i % 3 {
 		case 0: // distinct IDs
-			eActual = makeEndpoint(1, uint64(i), uint64(i))
-			eDesired = makeEndpoint(2, uint64(i), uint64(i))
+			eActual = makeEndpoint(1, i, i)
+			eDesired = makeEndpoint(2, i, i)
 		case 1: // same ID, same checksum
-			e := makeEndpoint(3, uint64(i), uint64(i))
+			e := makeEndpoint(3, i, i)
 			eActual = e
 			eDesired = e
 		case 2: // same ID, different checksum
-			eActual = makeEndpoint(4, uint64(i), uint64(i))
-			eDesired = makeEndpoint(4, uint64(i), uint64(i+1))
+			eActual = makeEndpoint(4, i, i)
+			eDesired = makeEndpoint(4, i, i+1)
 		}
 		for _, ch := range chActual {
 			ch <- eActual

pkg/icingadb/types/notification_type.go

@@ -3,7 +3,6 @@ package types
 import (
 	"database/sql/driver"
 	"encoding"
-	"github.com/icinga/icinga-go-library/types"
 	"github.com/pkg/errors"
 	"strconv"
 )
@@ -15,17 +14,12 @@ type NotificationType uint16
 func (nt *NotificationType) UnmarshalText(text []byte) error {
 	s := string(text)
 
-	i, err := strconv.ParseUint(s, 10, 64)
+	i, err := strconv.ParseUint(s, 10, 16)
 	if err != nil {
-		return types.CantParseUint64(err, s)
+		return errors.Wrapf(err, "can't parse %q into uint16", s)
 	}
 
 	n := NotificationType(i)
-	if uint64(n) != i {
-		// Truncated due to above cast, obviously too high
-		return badNotificationType(s)
-	}
-
 	if _, ok := notificationTypes[n]; !ok {
 		return badNotificationType(s)
 	}