🔧 Fix or add comments about gosec overflow errors (#496)

<!--
Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors.
All rights reserved.
SPDX-License-Identifier: Apache-2.0
-->
### Description

<!--
Please add any detail or context that would be useful to a reviewer.
-->

Fix or add comments about gosec overflow errors and add out of range
error to commonerrors.

### Test Coverage

<!--
Please put an `x` in the correct box e.g. `[x]` to indicate the testing
coverage of this change.
-->

- [x]  This change is covered by existing or additional automated tests.
- [ ] Manual testing has been performed (and evidence provided) as
automated testing was not feasible.
- [ ] Additional tests are not required for this change (e.g.
documentation update).

changes/20240827131009.feature

@@ -0,0 +1 @@
+:sparkles: Add out of range error to commonerrors

changes/20240827131050.misc

@@ -0,0 +1 @@
+:spanner: Fix or add comments about gosec overflow errors

utils/commonerrors/errors.go

@@ -44,6 +44,7 @@ var (
 	ErrCondition          = errors.New("failed condition")
 	ErrEOF                = errors.New("end of file")
 	ErrMalicious          = errors.New("suspected malicious intent")
+	ErrOutOfRange         = errors.New("out of range")
 	// ErrWarning is a generic error that can be used when an error should be raised but it shouldn't necessary be
 	// passed up the chain, for example in cases where an error should be logged but the program should continue. In
 	// these situations it should be handled immediately and then ignored/set to nil.
@@ -56,7 +57,7 @@ var warningStrPrepend = fmt.Sprintf("%v: ", warningStr)
 
 // IsCommonError returns whether an error is a commonerror
 func IsCommonError(target error) bool {
-	return Any(target, ErrNotImplemented, ErrNoExtension, ErrNoLogger, ErrNoLoggerSource, ErrNoLogSource, ErrUndefined, ErrInvalidDestination, ErrTimeout, ErrLocked, ErrStaleLock, ErrExists, ErrNotFound, ErrUnsupported, ErrUnavailable, ErrWrongUser, ErrUnauthorised, ErrUnknown, ErrInvalid, ErrConflict, ErrMarshalling, ErrCancelled, ErrEmpty, ErrUnexpected, ErrTooLarge, ErrForbidden, ErrCondition, ErrEOF, ErrMalicious, ErrWarning)
+	return Any(target, ErrNotImplemented, ErrNoExtension, ErrNoLogger, ErrNoLoggerSource, ErrNoLogSource, ErrUndefined, ErrInvalidDestination, ErrTimeout, ErrLocked, ErrStaleLock, ErrExists, ErrNotFound, ErrUnsupported, ErrUnavailable, ErrWrongUser, ErrUnauthorised, ErrUnknown, ErrInvalid, ErrConflict, ErrMarshalling, ErrCancelled, ErrEmpty, ErrUnexpected, ErrTooLarge, ErrForbidden, ErrCondition, ErrEOF, ErrMalicious, ErrWarning, ErrOutOfRange)
 }
 
 // Any determines whether the target error is of the same type as any of the errors `err`
@@ -180,6 +181,8 @@ func deserialiseCommonError(errStr string) (bool, error) {
 		return true, ErrMalicious
 	case CorrespondTo(ErrWarning, errStr):
 		return true, ErrWarning
+	case CorrespondTo(ErrOutOfRange, errStr):
+		return true, ErrOutOfRange
 	}
 	return false, ErrUnknown
 }

utils/commonerrors/errors_test.go

@@ -104,6 +104,7 @@ func TestIsCommonError(t *testing.T) {
 		ErrEOF,
 		ErrMalicious,
 		ErrWarning,
+		ErrOutOfRange,
 	}
 	for i := range commonErrors {
 		assert.True(t, IsCommonError(commonErrors[i]))

utils/field/fields_test.go

@@ -52,7 +52,7 @@ func TestOptionalField(t *testing.T) {
 		},
 		{
 			fieldType:    "Int32",
-			value:        int32(time.Now().Second()),
+			value:        int32(time.Now().Second()), //nolint:gosec // this should be okay until 2038
 			defaultValue: int32(97894),
 			setFunction: func(a any) any {
 				return ToOptionalInt32(a.(int32))
@@ -67,7 +67,7 @@ func TestOptionalField(t *testing.T) {
 		},
 		{
 			fieldType:    "UInt32",
-			value:        uint32(time.Now().Second()),
+			value:        uint32(time.Now().Second()), //nolint:gosec // this should be okay until 2038
 			defaultValue: uint32(97894),
 			setFunction: func(a any) any {
 				return ToOptionalUint32(a.(uint32))

utils/filesystem/files_test.go

@@ -76,7 +76,7 @@ func TestOpen(t *testing.T) {
 			// According to documentation, if the file does not exist, and the O_CREATE flag
 			//// is passed, it is created with mode perm (before umask)
 			mode := 0600
-			file, err := fs.OpenFile(filePath, os.O_RDWR|os.O_CREATE, os.FileMode(mode))
+			file, err := fs.OpenFile(filePath, os.O_RDWR|os.O_CREATE, os.FileMode(mode)) //nolint:gosec // this is a test and mode is 0600
 			require.NoError(t, err)
 			defer func() { _ = file.Close() }()
 
@@ -90,7 +90,7 @@ func TestOpen(t *testing.T) {
 			testFileMode(t, fs, filePath, mode)
 
 			ignoredMode := 0322
-			file, err = fs.OpenFile(filePath, os.O_WRONLY|os.O_APPEND, os.FileMode(ignoredMode))
+			file, err = fs.OpenFile(filePath, os.O_WRONLY|os.O_APPEND, os.FileMode(ignoredMode)) //nolint:gosec // this is a test and mode is 0322
 			require.NoError(t, err)
 			defer func() { _ = file.Close() }()
 
@@ -101,7 +101,7 @@ func TestOpen(t *testing.T) {
 			testFileMode(t, fs, filePath, mode)
 
 			ignoredMode = 0400
-			file, err = fs.OpenFile(filePath, os.O_RDONLY, os.FileMode(ignoredMode))
+			file, err = fs.OpenFile(filePath, os.O_RDONLY, os.FileMode(ignoredMode)) //nolint:gosec // this is a test and mode is 0400
 			require.NoError(t, err)
 			defer func() { _ = file.Close() }()
 
@@ -114,7 +114,7 @@ func TestOpen(t *testing.T) {
 			testFileMode(t, fs, filePath, mode)
 
 			ignoredMode = 0664
-			file, err = fs.OpenFile(filePath, os.O_RDWR|os.O_TRUNC, os.FileMode(ignoredMode))
+			file, err = fs.OpenFile(filePath, os.O_RDWR|os.O_TRUNC, os.FileMode(ignoredMode)) //nolint:gosec // this is a test and mode is 0664
 			require.NoError(t, err)
 			defer func() { _ = file.Close() }()
 
@@ -319,9 +319,9 @@ func TestChmod(t *testing.T) {
 			require.True(t, fs.Exists(tmpFile))
 			for _, mode := range []int{0666, 0777, 0555, 0766, 0444, 0644} {
 				t.Run(fmt.Sprintf("%v", mode), func(t *testing.T) {
-					err = fs.Chmod(tmpFile, os.FileMode(mode))
+					err = fs.Chmod(tmpFile, os.FileMode(mode)) //nolint:gosec // this is a test and mode can be seen above to abide by the conversion rules
 					if err != nil {
-						_ = fs.Chmod(tmpFile, os.FileMode(mode))
+						_ = fs.Chmod(tmpFile, os.FileMode(mode)) //nolint:gosec // this is a test and mode can be seen above to abide by the conversion rules
 					}
 					require.NoError(t, err)
 					testFileMode(t, fs, tmpFile, mode)
@@ -351,12 +351,12 @@ func TestChmodRecursive(t *testing.T) {
 			require.True(t, fs.Exists(tmpFile))
 			for _, mode := range []int{0666, 0777, 0555, 0766, 0444, 0644} {
 				t.Run(fmt.Sprintf("%v", mode), func(t *testing.T) {
-					err = fs.ChmodRecursively(context.TODO(), tmpFile2, os.FileMode(mode))
+					err = fs.ChmodRecursively(context.TODO(), tmpFile2, os.FileMode(mode)) //nolint:gosec // this is a test and mode can be seen above to abide by the conversion rules
 					if err == nil {
 						require.NoError(t, err)
 						testFileMode(t, fs, tmpFile2, mode)
 
-						err = fs.ChmodRecursively(context.TODO(), tmpDir, os.FileMode(mode))
+						err = fs.ChmodRecursively(context.TODO(), tmpDir, os.FileMode(mode)) //nolint:gosec // this is a test and mode can be seen above to abide by the conversion rules
 						if err == nil {
 							require.NoError(t, err)
 							testFileMode(t, fs, tmpFile, mode)

utils/filesystem/zip.go

@@ -4,6 +4,7 @@ import (
 	"archive/zip"
 	"context"
 	"fmt"
+	"math"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -364,8 +365,8 @@ func (fs *VFS) unzip(ctx context.Context, source string, destination string, lim
 		if limits.Apply() && totalSizeOnDisk.Load() > limits.GetMaxTotalSize() {
 			return fileList, fileCounter.Load(), totalSizeOnDisk.Load(), fmt.Errorf("%w: more than %v B of disk space was used while unzipping %v (%v B used already)", commonerrors.ErrTooLarge, limits.GetMaxTotalSize(), source, totalSizeOnDisk.Load())
 		}
-		if limits.Apply() && int64(fileCounter.Load()) > limits.GetMaxFileCount() {
-			return fileList, fileCounter.Load(), totalSizeOnDisk.Load(), fmt.Errorf("%w: more than %v files were created while unzipping %v (%v files created already)", commonerrors.ErrTooLarge, limits.GetMaxFileCount(), source, fileCounter.Load())
+		if filecount := fileCounter.Load(); limits.Apply() && filecount <= math.MaxInt64 && int64(filecount) > limits.GetMaxFileCount() { //nolint:gosec // if filecount of uint64 is greater than the max value of int64 then it must be greater than GetMaxFileCount as that is an int64
+			return fileList, filecount, totalSizeOnDisk.Load(), fmt.Errorf("%w: more than %v files were created while unzipping %v (%v files created already)", commonerrors.ErrTooLarge, limits.GetMaxFileCount(), source, filecount)
 		}
 	}
 

utils/platform/os.go

@@ -7,6 +7,7 @@ package platform
 import (
 	"errors"
 	"fmt"
+	"math"
 	"os"
 	"regexp"
 	"runtime"
@@ -109,7 +110,11 @@ func UpTime() (uptime time.Duration, err error) {
 	if err != nil {
 		return
 	}
-	uptime = time.Duration(_uptime) * time.Second
+	if _uptime > math.MaxInt64 { // The upper limit for time as defined by the standard library https://cs.opensource.google/go/go/+/master:src/time/time.go;l=915 is the same as math.MaxInt64
+		err = fmt.Errorf("%w: could not convert uptime '%v' to duration as it exceeds the upper limit for time.Duration", commonerrors.ErrOutOfRange, _uptime)
+		return
+	}
+	uptime = time.Duration(_uptime) * time.Second //nolint:gosec // we have verified the value of _uptime is whithin the upper limit for time.Duration in the above check
 	return
 }
 
@@ -119,7 +124,11 @@ func BootTime() (bootime time.Time, err error) {
 	if err != nil {
 		return
 	}
-	bootime = time.Unix(int64(_bootime), 0)
+	if _bootime > math.MaxInt64 { // The upper limit for time as defined by the standard library https://cs.opensource.google/go/go/+/master:src/time/time.go;l=915 is the same as math.MaxInt64
+		err = fmt.Errorf("%w: could not convert uptime '%v' to duration as it exceeds the upper limit for time.Duration", commonerrors.ErrOutOfRange, _bootime)
+		return
+	}
+	bootime = time.Unix(int64(_bootime), 0) //nolint:gosec // we have verified the value of _bootime is whithin the upper limit for time.Duration in the above check
 	return
 
 }
@@ -231,27 +240,27 @@ func SubstituteParameter(parameter ...string) string {
 // - the first element is the parameter to substitute
 // - if find and replace is also wanted, pass the pattern and the replacement as following arguments in that order.
 func SubstituteParameterUnix(parameter ...string) string {
-	if len(parameter) < 1 || !isUnixVarName(parameter[0]) {
+	if len(parameter) < 1 || !isUnixVarName(parameter[0]) { //nolint:gosec // golang order of evaluation means that this won't be out of range
 		return "${}"
 	}
 	if len(parameter) < 3 || parameter[1] == "" {
 		return fmt.Sprintf("${%v}", parameter[0])
 	}
-	return fmt.Sprintf("${%v//%v/%v}", parameter[0], parameter[1], parameter[2])
+	return fmt.Sprintf("${%v//%v/%v}", parameter[0], parameter[1], parameter[2]) //nolint:gosec // by this point in the code parameter is verified to have at least 3 elements
 }
 
 // SubstituteParameterWindows performs Windows parameter substitution:
 // See https://ss64.com/nt/syntax-replace.html
 // - the first element is the parameter to substitute
 // - if find and replace is also wanted, pass the pattern and the replacement as following arguments in that order.
 func SubstituteParameterWindows(parameter ...string) string {
-	if len(parameter) < 1 || !isWindowsVarName(parameter[0]) {
+	if len(parameter) < 1 || !isWindowsVarName(parameter[0]) { //nolint:gosec // golang order of evaluation means that this won't be out of range
 		return "%%"
 	}
 	if len(parameter) < 3 || parameter[1] == "" {
 		return "%" + parameter[0] + "%"
 	}
-	return "%" + fmt.Sprintf("%v:%v=%v", parameter[0], parameter[1], parameter[2]) + "%"
+	return "%" + fmt.Sprintf("%v:%v=%v", parameter[0], parameter[1], parameter[2]) + "%" //nolint:gosec // by this point in the code parameter is verified to have at least 3 elements
 }
 
 // ExpandParameter expands a variable expressed in a string `s` with its value returned by the mapping function.

utils/proc/process.go

@@ -245,7 +245,7 @@ func isProcessRunning(p *process.Process) (running bool) {
 // to get more information about the process. An error will be returned
 // if the process does not exist.
 func NewProcess(ctx context.Context, pid int) (pr IProcess, err error) {
-	p, err := process.NewProcessWithContext(ctx, int32(pid))
+	p, err := process.NewProcessWithContext(ctx, int32(pid)) //nolint:gosec // Max PID is 2^22 which is within int32 range https://stackoverflow.com/a/6294196
 	err = ConvertProcessError(err)
 	if err != nil {
 		return

utils/reflection/reflection.go

@@ -20,7 +20,9 @@ func GetStructureField(field reflect.Value) interface{} {
 	if !field.IsValid() {
 		return nil
 	}
-	return reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).Elem().Interface()
+	return reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())). //nolint:gosec // this conversion is is between types recommended by Go https://cs.opensource.google/go/go/+/master:src/reflect/value.go;l=2445
+										Elem().
+										Interface()
 }
 func SetUnexportedStructureField(structure interface{}, fieldName string, value interface{}) {
 	SetStructureField(fetchStructureField(structure, fieldName), value)
@@ -29,9 +31,9 @@ func SetStructureField(field reflect.Value, value interface{}) {
 	if !field.IsValid() {
 		return
 	}
-	reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).
-		Elem().
-		Set(reflect.ValueOf(value))
+	reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())). //nolint:gosec // this conversion is is between types recommended by Go https://cs.opensource.google/go/go/+/master:src/reflect/value.go;l=2445
+										Elem().
+										Set(reflect.ValueOf(value))
 }
 
 func fetchStructureField(structure interface{}, fieldName string) reflect.Value {