From 9535c9e3e1ae46d0aa6668c53c432ebdd10319d1 Mon Sep 17 00:00:00 2001 From: Nanik Date: Wed, 28 Jul 2021 06:03:59 +1000 Subject: [PATCH] fix: add variable assignment checking as part of MinVersion (#669) * fix: add variable assignment checking as part of MinVersion * fix: add more code to allow assignment with const * fix: rework the code and add more test cases for MinVersion * fix: format linting issue using gofumpt --- rules/tls.go | 26 ++++++++- testutils/source.go | 138 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 154 insertions(+), 10 deletions(-) diff --git a/rules/tls.go b/rules/tls.go index 219d8fc..486b56e 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -20,6 +20,8 @@ import ( "crypto/tls" "fmt" "go/ast" + "go/types" + "strconv" "github.com/securego/gosec/v2" ) @@ -85,7 +87,29 @@ func (t *insecureConfigTLS) processTLSConfVal(n *ast.KeyValueExpr, c *gosec.Cont } case "MinVersion": - if ival, ierr := gosec.GetInt(n.Value); ierr == nil { + if d, ok := n.Value.(*ast.Ident); ok { + if vs, ok := d.Obj.Decl.(*ast.ValueSpec); ok { + if s, ok := vs.Values[0].(*ast.SelectorExpr); ok { + x := s.X.(*ast.Ident).Name + sel := s.Sel.Name + + for _, imp := range c.Pkg.Imports() { + if imp.Name() == x { + tObj := imp.Scope().Lookup(sel) + if cst, ok := tObj.(*types.Const); ok { + // ..got the value check if this can be translated + if minVersion, err := strconv.ParseInt(cst.Val().String(), 10, 64); err == nil { + t.actualMinVersion = minVersion + } + } + } + } + } + if ival, ierr := gosec.GetInt(vs.Values[0]); ierr == nil { + t.actualMinVersion = ival + } + } + } else if ival, ierr := gosec.GetInt(n.Value); ierr == nil { t.actualMinVersion = ival } else { if se, ok := n.Value.(*ast.SelectorExpr); ok { diff --git a/testutils/source.go b/testutils/source.go index dba9612..5ce6ad2 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -2091,7 +2091,8 @@ func main() { } // SampleCodeG402 - TLS settings - SampleCodeG402 = []CodeSample{{[]string{` + SampleCodeG402 = []CodeSample{ + {[]string{` // InsecureSkipVerify package main import ( @@ -2109,8 +2110,9 @@ func main() { if err != nil { fmt.Println(err) } -}`}, 1, gosec.NewConfig()}, {[]string{ - ` +}`}, 1, gosec.NewConfig()}, + {[]string{ + ` // Insecure minimum version package main import ( @@ -2128,7 +2130,121 @@ func main() { fmt.Println(err) } }`, - }, 1, gosec.NewConfig()}, {[]string{` + }, 1, gosec.NewConfig()}, + {[]string{ + ` +// Insecure minimum version +package main +import ( + "crypto/tls" + "fmt" +) + +func CaseNotError() *tls.Config { + var v uint16 = tls.VersionTLS13 + + return &tls.Config{ + MinVersion: v, + } +} + +func main() { + a := CaseNotError() + fmt.Printf("Debug: %v\n", a.MinVersion) +}`, + }, 0, gosec.NewConfig()}, + {[]string{ + ` +// Insecure minimum version +package main +import ( + "crypto/tls" + "fmt" +) + +func CaseNotError() *tls.Config { + return &tls.Config{ + MinVersion: tls.VersionTLS13, + } +} + +func main() { + a := CaseNotError() + fmt.Printf("Debug: %v\n", a.MinVersion) +}`, + }, 0, gosec.NewConfig()}, + {[]string{ + ` +// Insecure minimum version +package main +import ( + "crypto/tls" + "fmt" +) + +func CaseError() *tls.Config { + var v = &tls.Config{ + MinVersion: 0, + } + return v +} + +func main() { + a := CaseError() + fmt.Printf("Debug: %v\n", a.MinVersion) +}`, + }, 1, gosec.NewConfig()}, + {[]string{ + ` +// Insecure minimum version +package main +import ( + "crypto/tls" + "fmt" +) + +func CaseError() *tls.Config { + var v = &tls.Config{ + MinVersion: getVersion(), + } + return v +} + +func getVersion() uint16 { + return tls.VersionTLS12 +} + +func main() { + a := CaseError() + fmt.Printf("Debug: %v\n", a.MinVersion) +}`, + }, 1, gosec.NewConfig()}, + {[]string{ + ` +// Insecure minimum version +package main + +import ( + "crypto/tls" + "fmt" + "net/http" +) + +var theValue uint16 = 0x0304 + +func main() { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{MinVersion: theValue}, + } + client := &http.Client{Transport: tr} + _, err := client.Get("https://golang.org/") + if err != nil { + fmt.Println(err) + } +} +`, + }, 0, gosec.NewConfig()}, + {[]string{` // Insecure max version package main import ( @@ -2146,8 +2262,9 @@ func main() { fmt.Println(err) } } -`}, 1, gosec.NewConfig()}, { - []string{` +`}, 1, gosec.NewConfig()}, + { + []string{` // Insecure ciphersuite selection package main import ( @@ -2168,7 +2285,8 @@ func main() { fmt.Println(err) } }`}, 1, gosec.NewConfig(), - }, {[]string{` + }, + {[]string{` // secure max version when min version is specified package main import ( @@ -2185,7 +2303,8 @@ func main() { if err != nil { fmt.Println(err) } -}`}, 0, gosec.NewConfig()}, {[]string{` +}`}, 0, gosec.NewConfig()}, + {[]string{` package p0 import "crypto/tls" @@ -2202,7 +2321,8 @@ import "crypto/tls" func TlsConfig1() *tls.Config { return &tls.Config{MinVersion: 0x0304} } -`}, 1, gosec.NewConfig()}} +`}, 1, gosec.NewConfig()}, + } // SampleCodeG403 - weak key strength SampleCodeG403 = []CodeSample{