diff --git a/changes/20240827131009.feature b/changes/20240827131009.feature new file mode 100644 index 0000000000..5500c9a176 --- /dev/null +++ b/changes/20240827131009.feature @@ -0,0 +1 @@ +:sparkles: Add out of range error to commonerrors diff --git a/changes/20240827131050.misc b/changes/20240827131050.misc new file mode 100644 index 0000000000..79712de04e --- /dev/null +++ b/changes/20240827131050.misc @@ -0,0 +1 @@ +:spanner: Fix or add comments about gosec overflow errors diff --git a/utils/commonerrors/errors.go b/utils/commonerrors/errors.go index d2125babc9..692934c7ed 100644 --- a/utils/commonerrors/errors.go +++ b/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 } diff --git a/utils/commonerrors/errors_test.go b/utils/commonerrors/errors_test.go index 3be578c642..a9e27d2063 100644 --- a/utils/commonerrors/errors_test.go +++ b/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])) diff --git a/utils/field/fields_test.go b/utils/field/fields_test.go index 9f26f24553..6cde37dcd3 100644 --- a/utils/field/fields_test.go +++ b/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)) diff --git a/utils/filesystem/files_test.go b/utils/filesystem/files_test.go index 34c7a73333..e5b1ddb8df 100644 --- a/utils/filesystem/files_test.go +++ b/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) diff --git a/utils/filesystem/zip.go b/utils/filesystem/zip.go index 21a942ede5..4ab335b78e 100644 --- a/utils/filesystem/zip.go +++ b/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) } } diff --git a/utils/platform/os.go b/utils/platform/os.go index 1617a7492b..5c88173087 100644 --- a/utils/platform/os.go +++ b/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,13 +240,13 @@ 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: @@ -245,13 +254,13 @@ func SubstituteParameterUnix(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 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. diff --git a/utils/proc/process.go b/utils/proc/process.go index 3cac1cc6bf..af425b886d 100644 --- a/utils/proc/process.go +++ b/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 diff --git a/utils/reflection/reflection.go b/utils/reflection/reflection.go index 68b3ecd647..0ad0b96a52 100644 --- a/utils/reflection/reflection.go +++ b/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 {