Skip to content

Commit

Permalink
Ignore struct pointers in G601 (#1003)
Browse files Browse the repository at this point in the history
Updates #966

Signed-off-by: Alexander Yastrebov <[email protected]>
  • Loading branch information
AlexanderYastrebov authored Aug 18, 2023
1 parent 85005c4 commit 21d13c9
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 21 deletions.
7 changes: 3 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ endif
BUILDFLAGS := "-w -s -X 'main.Version=$(GIT_TAG)' -X 'main.GitTag=$(GIT_TAG)' -X 'main.BuildDate=$(BUILD_DATE)'"
CGO_ENABLED = 0
GO := GO111MODULE=on go
GO_NOMOD :=GO111MODULE=off go
GOPATH ?= $(shell $(GO) env GOPATH)
GOBIN ?= $(GOPATH)/bin
GOSEC ?= $(GOBIN)/gosec
Expand All @@ -25,8 +24,8 @@ default:

install-test-deps:
go install github.com/onsi/ginkgo/v2/ginkgo@latest
$(GO_NOMOD) get -u golang.org/x/crypto/ssh
$(GO_NOMOD) get -u github.com/lib/pq
go install golang.org/x/crypto/...@latest
go install github.com/lib/pq/...@latest

install-govulncheck:
@if [ $(GO_MINOR_VERSION) -gt $(GOVULN_MIN_VERSION) ]; then \
Expand Down Expand Up @@ -89,5 +88,5 @@ image-push: image

tlsconfig:
go generate ./...

.PHONY: test build clean release image image-push tlsconfig
30 changes: 16 additions & 14 deletions rules/implicit_aliasing.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package rules
import (
"go/ast"
"go/token"
"go/types"

"github.com/securego/gosec/v2"
"github.com/securego/gosec/v2/issue"
Expand All @@ -28,23 +29,20 @@ func containsUnary(exprs []*ast.UnaryExpr, expr *ast.UnaryExpr) bool {
return false
}

func getIdentExpr(expr ast.Expr) *ast.Ident {
func getIdentExpr(expr ast.Expr) (*ast.Ident, bool) {
return doGetIdentExpr(expr, false)
}

func doGetIdentExpr(expr ast.Expr, hasSelector bool) (*ast.Ident, bool) {
switch node := expr.(type) {
case *ast.Ident:
return node
return node, hasSelector
case *ast.SelectorExpr:
return getIdentExpr(node.X)
return doGetIdentExpr(node.X, true)
case *ast.UnaryExpr:
switch e := node.X.(type) {
case *ast.Ident:
return e
case *ast.SelectorExpr:
return getIdentExpr(e.X)
default:
return nil
}
return doGetIdentExpr(node.X, hasSelector)
default:
return nil
return nil, false
}
}

Expand Down Expand Up @@ -92,9 +90,13 @@ func (r *implicitAliasing) Match(n ast.Node, c *gosec.Context) (*issue.Issue, er
}

// If we find a unary op of & (reference) of an object within r.aliases, complain.
if identExpr := getIdentExpr(node); identExpr != nil && node.Op.String() == "&" {
if identExpr, hasSelector := getIdentExpr(node); identExpr != nil && node.Op.String() == "&" {
if _, contains := r.aliases[identExpr.Obj]; contains {
return c.NewIssue(n, r.ID(), r.What, r.Severity, r.Confidence), nil
_, isPointer := c.Info.TypeOf(identExpr).(*types.Pointer)

if !hasSelector || !isPointer {
return c.NewIssue(n, r.ID(), r.What, r.Severity, r.Confidence), nil
}
}
}
case *ast.ReturnStmt:
Expand Down
84 changes: 81 additions & 3 deletions testutils/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ func HelloServer(w http.ResponseWriter, r *http.Request) {
"fmt"
"net/http"
)
func main() {
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintf(w, "Hello, %s!", r.URL.Path[1:])
Expand All @@ -1199,7 +1199,7 @@ func HelloServer(w http.ResponseWriter, r *http.Request) {
"time"
"net/http"
)
func main() {
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintf(w, "Hello, %s!", r.URL.Path[1:])
Expand All @@ -1222,7 +1222,7 @@ func HelloServer(w http.ResponseWriter, r *http.Request) {
"time"
"net/http"
)
func main() {
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintf(w, "Hello, %s!", r.URL.Path[1:])
Expand Down Expand Up @@ -3623,6 +3623,46 @@ type sampleStruct struct {
name string
}
func main() {
samples := []*sampleStruct{
{name: "a"},
{name: "b"},
}
for _, sample := range samples {
fmt.Println(&sample)
}
}`}, 1, gosec.NewConfig()},
{[]string{`
package main
import (
"fmt"
)
type sampleStruct struct {
name string
}
func main() {
samples := []*sampleStruct{
{name: "a"},
{name: "b"},
}
for _, sample := range samples {
fmt.Println(&sample.name)
}
}`}, 0, gosec.NewConfig()},
{[]string{`
package main
import (
"fmt"
)
type sampleStruct struct {
name string
}
func main() {
samples := []sampleStruct{
{name: "a"},
Expand Down Expand Up @@ -3655,6 +3695,44 @@ func main() {
for _, sample := range samples {
fmt.Println(&sample.sub.name)
}
}`}, 1, gosec.NewConfig()},
{[]string{`
package main
import (
"fmt"
)
type subStruct struct {
name string
}
type sampleStruct struct {
sub subStruct
}
func main() {
samples := []*sampleStruct{
{sub: subStruct{name: "a"}},
{sub: subStruct{name: "b"}},
}
for _, sample := range samples {
fmt.Println(&sample.sub.name)
}
}`}, 0, gosec.NewConfig()},
{[]string{`
package main
import (
"fmt"
)
func main() {
one, two := 1, 2
samples := []*int{&one, &two}
for _, sample := range samples {
fmt.Println(&sample)
}
}`}, 1, gosec.NewConfig()},
}

Expand Down

0 comments on commit 21d13c9

Please sign in to comment.