Skip to content

Commit

Permalink
Add exemptions and/or checks for G115.
Browse files Browse the repository at this point in the history
  • Loading branch information
doriable committed Sep 24, 2024
1 parent 3fb800a commit f2173f1
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 2 deletions.
55 changes: 55 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,58 @@ issues:
# trip this off.
path: private/pkg/oauth2/device.go
text: "G101:"
# G115 checks for integer overflow from integer conversions. There are known false
# positives from the check (https://github.com/securego/gosec/issues/1212) that are
# actively being worked on. Each exemption below is a false positive or for a safe operation,
# such as parsing indices from descriptors and/or images.
- linters:
- gosec
# Loop index conversion to uint64.
path: private/buf/bufgen/features.go
text: "G115:"
- linters:
- gosec
# Converting result from utf8.RuneCountInString to uint64.
path: private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go
text: "G115:"
- linters:
- gosec
# PluginReference revision is validated with a bounds check at construction time.
path: private/bufpkg/bufremoteplugin/bufremoteplugin.go
text: "G115:"
- linters:
- gosec
# A bounds check has been added for int32 -> uint32 conversion this is being flagged
# as a false positive.
path: private/buf/bufcurl/reflection_resolver.go
text: "G115:"
- linters:
- gosec
# bufprotosource converts indices to int32 to form the source path. Since it is parsing
# from the fileDescriptor set, the operation should be safe.
path: private/bufpkg/bufprotosource/paths.go
text: "G115:"
- linters:
- gosec
# bufimageutil is handling images and converting loop indices to int32. Since it is
# parsing from an Image, the operation should be safe.
path: private/bufpkg/bufimage/bufimageutil/bufimageutil.go
text: "G115:"
- linters:
- gosec
# Bounds checks have been added with assertion statements to ensure safe int -> int32
# conversions, this is a false positive.
path: private/bufpkg/bufprotosource/option_extension_descriptor_test.go
text: "G115:"
- linters:
- gosec
# This converts results from strconv.ParseInt with the bit size set to 32 to int32,
# so it should be a safe conversion, this is a false positive.
path: private/buf/bufprotopluginexec/version.go
text: "G115:"
- linters:
- gosec
# This checks the cel constraints from an Image, and converts loop indices to int32
# to set the source path for the location, this operation should be safe.
path: private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go
text: "G115:"
10 changes: 10 additions & 0 deletions private/buf/bufcurl/reflection_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ func (r *reflectionResolver) ListServices() ([]protoreflect.FullName, error) {
}
switch response := resp.MessageResponse.(type) {
case *reflectionv1.ServerReflectionResponse_ErrorResponse:
// This should never happen, however we do a bounds check to ensure we are doing a safe
// conversion from int32 (ErrorResponse.ErrorCode) to uint32 (connect.Code).
if response.ErrorResponse.ErrorCode < 0 {
return nil, fmt.Errorf("server replied with unsupported error code: %v", response.ErrorResponse.ErrorCode)
}
return nil, connect.NewWireError(connect.Code(response.ErrorResponse.ErrorCode), errors.New(response.ErrorResponse.ErrorMessage))
case *reflectionv1.ServerReflectionResponse_ListServicesResponse:
serviceNames := make([]protoreflect.FullName, len(response.ListServicesResponse.Service))
Expand Down Expand Up @@ -338,6 +343,11 @@ func (r *reflectionResolver) fileByNameLocked(name string) ([]*descriptorpb.File
func descriptorsInResponse(resp *reflectionv1.ServerReflectionResponse) ([]*descriptorpb.FileDescriptorProto, error) {
switch response := resp.MessageResponse.(type) {
case *reflectionv1.ServerReflectionResponse_ErrorResponse:
// This should never happen, however we do a bounds check to ensure we are doing a safe
// conversion from int32 (ErrorResponse.ErrorCode) to uint32 (connect.Code).
if response.ErrorResponse.ErrorCode < 0 {
return nil, fmt.Errorf("server replied with unsupported error code: %v", response.ErrorResponse.ErrorCode)
}
return nil, connect.NewWireError(connect.Code(response.ErrorResponse.ErrorCode), errors.New(response.ErrorResponse.ErrorMessage))
case *reflectionv1.ServerReflectionResponse_FileDescriptorResponse:
files := make([]*descriptorpb.FileDescriptorProto, len(response.FileDescriptorResponse.FileDescriptorProto))
Expand Down
6 changes: 6 additions & 0 deletions private/bufpkg/bufimage/bufimage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"io/fs"
"math"
"slices"
"sort"
"strings"
Expand Down Expand Up @@ -703,6 +704,11 @@ func reparseImageProto(protoImage *imagev1.Image, resolver protoencoding.Resolve
}
}
if !isPublic {
// This should never happen, however we do a bounds check to ensure that we are
// doing a safe conversion for the index.
if i > math.MaxInt32 || i < math.MinInt32 {
return fmt.Errorf("unused dependency index out-of-bounds for int32 conversion: %v", i)
}
bufExt.UnusedDependency = append(bufExt.UnusedDependency, int32(i))
}
}
Expand Down
6 changes: 6 additions & 0 deletions private/bufpkg/bufimage/build_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"fmt"
"math"
"strings"

"github.com/bufbuild/buf/private/bufpkg/bufanalysis"
Expand Down Expand Up @@ -306,6 +307,11 @@ func getImageFilesRec(
dependency := fileDescriptor.Imports().Get(i).FileDescriptor
if unusedDependencyFilenames != nil {
if _, ok := unusedDependencyFilenames[dependency.Path()]; ok {
// This should never happen, however we do a bounds check to ensure that we are
// doing a safe conversion for the index.
if i > math.MaxInt32 || i < math.MinInt32 {
return nil, fmt.Errorf("unused dependency index out-of-bounds for int32 conversion: %v", i)
}
unusedDependencyIndexes = append(
unusedDependencyIndexes,
int32(i),
Expand Down
10 changes: 10 additions & 0 deletions private/bufpkg/bufprotosource/option_extension_descriptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package bufprotosource

import (
"math"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -121,10 +122,19 @@ func TestOptionExtensionLocation(t *testing.T) {
func checkLocation(t *testing.T, loc Location, sourceCodeInfoLoc *descriptorpb.SourceCodeInfo_Location) {
t.Helper()
assert.Equal(t, sourceCodeInfoLoc.GetLeadingComments(), loc.LeadingComments())
// Bounds assertions for int -> int32 conversion
assert.Less(t, loc.StartLine(), math.MaxInt32)
assert.Greater(t, loc.StartLine(), math.MinInt32)
assert.Less(t, loc.StartColumn(), math.MaxInt32)
assert.Greater(t, loc.StartLine(), math.MinInt32)
span := []int32{int32(loc.StartLine() - 1), int32(loc.StartColumn() - 1)}
if loc.EndLine() != loc.StartLine() {
assert.Less(t, loc.EndLine(), math.MaxInt32)
assert.Greater(t, loc.EndLine(), math.MinInt32)
span = append(span, int32(loc.EndLine()-1))
}
assert.Less(t, loc.EndColumn(), math.MaxInt32)
assert.Greater(t, loc.EndColumn(), math.MinInt32)
span = append(span, int32(loc.EndColumn()-1))
assert.Equal(t, sourceCodeInfoLoc.Span, span)
}
Expand Down
4 changes: 2 additions & 2 deletions private/pkg/normalpath/normalpath_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ func TestStripComponents(t *testing.T) {
testStripComponents(t, 5, "", false, "foo/bar/baz/bat")
}

func testStripComponents(t *testing.T, count int, expected string, expectedOK bool, path string) {
actual, ok := StripComponents(path, uint32(count))
func testStripComponents(t *testing.T, count uint32, expected string, expectedOK bool, path string) {
actual, ok := StripComponents(path, count)
assert.Equal(t, expectedOK, ok)
assert.Equal(t, expected, actual)
}
Expand Down

0 comments on commit f2173f1

Please sign in to comment.