fix: add variable assignment checking as part of MinVersion (securego…

…#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
dangolbeeker · Jul 27, 2021 · 9535c9e · 9535c9e
1 parent b869720
commit 9535c9e
Show file tree

Hide file tree

Showing 2 changed files with 154 additions and 10 deletions.
diff --git 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
@@ -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{