diff --git a/.circleci/config.yml b/.circleci/config.yml index 19cbed6..5b4c17f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -9,7 +9,7 @@ executors: - image: node:22-slim golangci-lint: docker: - - image: golangci/golangci-lint:v1.59 + - image: golangci/golangci-lint:v1.60 golang-previous: docker: - image: golang:1.22 diff --git a/.golangci.yml b/.golangci.yml index 2bd5d96..f7acdfd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,7 +16,6 @@ linters: - errchkjson - errname - errorlint - - exportloopref - forcetypeassert - gochecknoinits - gocritic diff --git a/pkg/sif/create.go b/pkg/sif/create.go index 0eb1e1d..0093f4f 100644 --- a/pkg/sif/create.go +++ b/pkg/sif/create.go @@ -12,33 +12,47 @@ import ( "errors" "fmt" "io" + "math" "os" "time" "github.com/google/uuid" ) +var errAlignmentOverflow = errors.New("integer overflow when calculating alignment") + // nextAligned finds the next offset that satisfies alignment. -func nextAligned(offset int64, alignment int) int64 { +func nextAligned(offset int64, alignment int) (int64, error) { align64 := uint64(alignment) offset64 := uint64(offset) - if align64 != 0 && offset64%align64 != 0 { - offset64 = (offset64 & ^(align64 - 1)) + align64 + if align64 <= 0 || offset64%align64 == 0 { + return offset, nil } - return int64(offset64) + offset64 += (align64 - offset64%align64) + + if offset64 > math.MaxInt64 { + return 0, errAlignmentOverflow + } + + //nolint:gosec // Overflow handled above. + return int64(offset64), nil } // writeDataObjectAt writes the data object described by di to ws, using time t, recording details // in d. The object is written at the first position that satisfies the alignment requirements // described by di following offsetUnaligned. func writeDataObjectAt(ws io.WriteSeeker, offsetUnaligned int64, di DescriptorInput, t time.Time, d *rawDescriptor) error { //nolint:lll - offset, err := ws.Seek(nextAligned(offsetUnaligned, di.opts.alignment), io.SeekStart) + offset, err := nextAligned(offsetUnaligned, di.opts.alignment) if err != nil { return err } + if _, err := ws.Seek(offset, io.SeekStart); err != nil { + return err + } + n, err := io.Copy(ws, di.r) if err != nil { return err @@ -72,6 +86,7 @@ func (f *FileImage) calculatedDataSize() int64 { var ( errInsufficientCapacity = errors.New("insufficient descriptor capacity to add data object(s) to image") errPrimaryPartition = errors.New("image already contains a primary partition") + errObjectIDOverflow = errors.New("object ID would overflow") ) // writeDataObject writes the data object described by di to f, using time t, recording details in @@ -81,6 +96,11 @@ func (f *FileImage) writeDataObject(i int, di DescriptorInput, t time.Time) erro return errInsufficientCapacity } + // We derive the ID from i, so make sure the ID will not overflow. + if i >= math.MaxInt32 { + return errObjectIDOverflow + } + // If this is a primary partition, verify there isn't another primary partition, and update the // architecture in the global header. if p, ok := di.opts.md.(partition); ok && p.Parttype == PartPrimSys { @@ -92,7 +112,7 @@ func (f *FileImage) writeDataObject(i int, di DescriptorInput, t time.Time) erro } d := &f.rds[i] - d.ID = uint32(i) + 1 + d.ID = uint32(i) + 1 //nolint:gosec // Overflow handled above. f.h.DataSize = f.calculatedDataSize() diff --git a/pkg/sif/create_test.go b/pkg/sif/create_test.go index fec085f..c374426 100644 --- a/pkg/sif/create_test.go +++ b/pkg/sif/create_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2024, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the // LICENSE file distributed with the sources of this project regarding your // rights to use or distribute this software. @@ -7,6 +7,7 @@ package sif import ( "errors" + "math" "os" "testing" "time" @@ -15,27 +16,34 @@ import ( ) func TestNextAligned(t *testing.T) { - cases := []struct { - name string - offset int64 - align int - expected int64 + tests := []struct { + name string + offset int64 + align int + wantOffset int64 + wantErr error }{ - {name: "align 0 to 0", offset: 0, align: 0, expected: 0}, - {name: "align 1 to 0", offset: 1, align: 0, expected: 1}, - {name: "align 0 to 1024", offset: 0, align: 1024, expected: 0}, - {name: "align 1 to 1024", offset: 1, align: 1024, expected: 1024}, - {name: "align 1023 to 1024", offset: 1023, align: 1024, expected: 1024}, - {name: "align 1024 to 1024", offset: 1024, align: 1024, expected: 1024}, - {name: "align 1025 to 1024", offset: 1025, align: 1024, expected: 2048}, + {name: "align 0 to 0", offset: 0, align: 0, wantOffset: 0}, + {name: "align 1 to 0", offset: 1, align: 0, wantOffset: 1}, + {name: "align 0 to 1024", offset: 0, align: 1024, wantOffset: 0}, + {name: "align 1 to 1024", offset: 1, align: 1024, wantOffset: 1024}, + {name: "align 1023 to 1024", offset: 1023, align: 1024, wantOffset: 1024}, + {name: "align 1024 to 1024", offset: 1024, align: 1024, wantOffset: 1024}, + {name: "align 1025 to 1024", offset: 1025, align: 1024, wantOffset: 2048}, + {name: "align max to 1024", offset: math.MaxInt64, align: 1024, wantErr: errAlignmentOverflow}, + {name: "align max to max", offset: math.MaxInt64, align: math.MaxInt - 1, wantErr: errAlignmentOverflow}, } - for _, tc := range cases { - actual := nextAligned(tc.offset, tc.align) - if actual != tc.expected { - t.Errorf("nextAligned case: %q, offset: %d, align: %d, expecting: %d, actual: %d\n", - tc.name, tc.offset, tc.align, tc.expected, actual) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + offset, err := nextAligned(tt.offset, tt.align) + if got, want := err, tt.wantErr; !errors.Is(got, want) { + t.Errorf("got error %v, want %v", got, want) + } + if got, want := offset, tt.wantOffset; got != want { + t.Errorf("got offset %v, want %v", got, want) + } + }) } } diff --git a/pkg/siftool/del.go b/pkg/siftool/del.go index 738aaa1..773a9b2 100644 --- a/pkg/siftool/del.go +++ b/pkg/siftool/del.go @@ -29,6 +29,7 @@ func (c *command) getDel() *cobra.Command { return fmt.Errorf("while converting id: %w", err) } + //nolint:gosec // ParseUint above ensures id is safe to cast to uint32. return c.app.Del(args[1], uint32(id)) }, DisableFlagsInUseLine: true, diff --git a/pkg/siftool/dump.go b/pkg/siftool/dump.go index 63379a4..d8b4d4e 100644 --- a/pkg/siftool/dump.go +++ b/pkg/siftool/dump.go @@ -30,6 +30,7 @@ func (c *command) getDump() *cobra.Command { return fmt.Errorf("while converting id: %w", err) } + //nolint:gosec // ParseUint above ensures id is safe to cast to uint32. return c.app.Dump(args[1], uint32(id)) }, DisableFlagsInUseLine: true, diff --git a/pkg/siftool/info.go b/pkg/siftool/info.go index 1093e37..8ee7a3b 100644 --- a/pkg/siftool/info.go +++ b/pkg/siftool/info.go @@ -31,6 +31,7 @@ func (c *command) getInfo() *cobra.Command { return fmt.Errorf("while converting id: %w", err) } + //nolint:gosec // ParseUint above ensures id is safe to cast to uint32. return c.app.Info(args[1], uint32(id)) }, DisableFlagsInUseLine: true, diff --git a/pkg/siftool/setprim.go b/pkg/siftool/setprim.go index 2ab2456..b455995 100644 --- a/pkg/siftool/setprim.go +++ b/pkg/siftool/setprim.go @@ -29,6 +29,7 @@ func (c *command) getSetPrim() *cobra.Command { return fmt.Errorf("while converting id: %w", err) } + //nolint:gosec // ParseUint above ensures id is safe to cast to uint32. return c.app.Setprim(args[1], uint32(id)) }, DisableFlagsInUseLine: true,