Refactor rules to use callListRule base structure (#1458)

* Refactor rules to utilize callListRule base structure

- Introduced a new base structure `callListRule` in `rules/base.go` to standardize the implementation of rules that check for specific function calls.
- Updated existing rules to inherit from `callListRule`, simplifying their structure and removing redundant ID methods.
- Modified the `MetaData` field to use `RuleID` instead of `ID` for consistency across rules.
- Removed the `weakcryptohash.go` and `weakdepricatedcryptohash.go` files as their functionality has been integrated into the new structure.

* fix(tlsconfig): correct MetaData field name in generated TLS check

* refactor: standardize rule metadata and call list initialization

Author: oittaa

cmd/tlsconfig/rule_template.go

@@ -7,7 +7,7 @@ var generatedRuleTmpl = template.Must(template.New("generated").Parse(`
 // DO NOT EDIT - generated by tlsconfig tool
 func New{{.Name}}TLSCheck(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	return &insecureConfigTLS{
-                MetaData: issue.MetaData{ID: id},
+                MetaData: issue.MetaData{RuleID: id},
 		requiredType: "crypto/tls.Config",
 		MinVersion:   {{ .MinVersion }},
 		MaxVersion:   {{ .MaxVersion }},

issue/issue.go

@@ -128,12 +128,28 @@ func (i *Issue) FileLocation() string {
 // MetaData is embedded in all gosec rules. The Severity, Confidence and What message
 // will be passed through to reported issues.
 type MetaData struct {
-	ID         string
+	RuleID     string
 	Severity   Score
 	Confidence Score
 	What       string
 }
 
+// NewMetaData creates a new MetaData object
+func NewMetaData(id, what string, severity, confidence Score) MetaData {
+	return MetaData{
+		RuleID:     id,
+		What:       what,
+		Severity:   severity,
+		Confidence: confidence,
+	}
+}
+
+// ID returns the rule ID. This satisfies part of the gosec.Rule interface
+// when MetaData is embedded in a rule struct.
+func (m MetaData) ID() string {
+	return m.RuleID
+}
+
 // MarshalJSON is used convert a Score object into a JSON representation
 func (c Score) MarshalJSON() ([]byte, error) {
 	return json.Marshal(c.String())

rules/archive.go

@@ -11,15 +11,10 @@ import (
 )
 
 type archive struct {
-	issue.MetaData
-	calls    gosec.CallList
+	callListRule
 	argTypes []string
 }
 
-func (a *archive) ID() string {
-	return a.MetaData.ID
-}
-
 // getArchiveBaseType returns the underlying type (*archive/zip.File or *archive/tar.Header)
 // if the expression is a direct .Name selector on such a type or a short-declared variable
 // assigned from such a selector (e.g., name := file.Name).
@@ -72,17 +67,10 @@ func (a *archive) Match(n ast.Node, ctx *gosec.Context) (*issue.Issue, error) {
 
 // NewArchive creates a new rule which detects file traversal when extracting zip/tar archives.
 func NewArchive(id string, _ gosec.Config) (gosec.Rule, []ast.Node) {
-	calls := gosec.NewCallList()
-	calls.Add("path/filepath", "Join")
-	calls.Add("path", "Join")
-	return &archive{
-		calls:    calls,
-		argTypes: []string{"*archive/zip.File", "*archive/tar.Header"},
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-			What:       "File traversal when extracting zip/tar archive",
-		},
-	}, []ast.Node{(*ast.CallExpr)(nil)}
+	rule := &archive{
+		callListRule: newCallListRule(id, "File traversal when extracting zip/tar archive", issue.Medium, issue.High),
+		argTypes:     []string{"*archive/zip.File", "*archive/tar.Header"},
+	}
+	rule.Add("path/filepath", "Join").Add("path", "Join")
+	return rule, []ast.Node{(*ast.CallExpr)(nil)}
 }

rules/base.go

@@ -0,0 +1,39 @@
+package rules
+
+import (
+	"go/ast"
+
+	"github.com/securego/gosec/v2"
+	"github.com/securego/gosec/v2/issue"
+)
+
+// callListRule is a base for rules that simply check a CallList and issue on match.
+// It provides the standard Match() implementation used by most call-based rules.
+type callListRule struct {
+	issue.MetaData
+	calls gosec.CallList
+}
+
+func newCallListRule(id, what string, severity, confidence issue.Score) callListRule {
+	return callListRule{
+		MetaData: issue.NewMetaData(id, what, severity, confidence),
+		calls:    gosec.NewCallList(),
+	}
+}
+
+func (r *callListRule) Add(selector, ident string) *callListRule {
+	r.calls.Add(selector, ident)
+	return r
+}
+
+func (r *callListRule) AddAll(selector string, idents ...string) *callListRule {
+	r.calls.AddAll(selector, idents...)
+	return r
+}
+
+func (r *callListRule) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) {
+	if r.calls.ContainsPkgCallExpr(n, c, false) != nil {
+		return c.NewIssue(n, r.ID(), r.What, r.Severity, r.Confidence), nil
+	}
+	return nil, nil
+}

rules/bind.go

@@ -24,15 +24,10 @@ import (
 
 // Looks for net.Listen("0.0.0.0") or net.Listen(":8080")
 type bindsToAllNetworkInterfaces struct {
-	issue.MetaData
-	calls   gosec.CallList
+	callListRule
 	pattern *regexp.Regexp
 }
 
-func (r *bindsToAllNetworkInterfaces) ID() string {
-	return r.MetaData.ID
-}
-
 func (r *bindsToAllNetworkInterfaces) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) {
 	callExpr := r.calls.ContainsPkgCallExpr(n, c, false)
 	if callExpr == nil {
@@ -68,17 +63,10 @@ func (r *bindsToAllNetworkInterfaces) Match(n ast.Node, c *gosec.Context) (*issu
 // NewBindsToAllNetworkInterfaces detects socket connections that are setup to
 // listen on all network interfaces.
 func NewBindsToAllNetworkInterfaces(id string, _ gosec.Config) (gosec.Rule, []ast.Node) {
-	calls := gosec.NewCallList()
-	calls.Add("net", "Listen")
-	calls.Add("crypto/tls", "Listen")
-	return &bindsToAllNetworkInterfaces{
-		calls:   calls,
-		pattern: regexp.MustCompile(`^(0.0.0.0|:).*$`),
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-			What:       "Binds to all network interfaces",
-		},
-	}, []ast.Node{(*ast.CallExpr)(nil)}
+	rule := &bindsToAllNetworkInterfaces{
+		callListRule: newCallListRule(id, "Binds to all network interfaces", issue.Medium, issue.High),
+		pattern:      regexp.MustCompile(`^(0.0.0.0|:).*$`),
+	}
+	rule.Add("net", "Listen").Add("crypto/tls", "Listen")
+	return rule, []ast.Node{(*ast.CallExpr)(nil)}
 }

rules/blocklist.go

@@ -33,10 +33,6 @@ func unquote(original string) string {
 	return strings.TrimRight(cleaned, `"`)
 }
 
-func (r *blocklistedImport) ID() string {
-	return r.MetaData.ID
-}
-
 func (r *blocklistedImport) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) {
 	if node, ok := n.(*ast.ImportSpec); ok {
 		if description, ok := r.Blocklisted[unquote(node.Path.Value)]; ok {
@@ -50,11 +46,7 @@ func (r *blocklistedImport) Match(n ast.Node, c *gosec.Context) (*issue.Issue, e
 // Typically when a deprecated technology is being used.
 func NewBlocklistedImports(id string, _ gosec.Config, blocklist map[string]string) (gosec.Rule, []ast.Node) {
 	return &blocklistedImport{
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-		},
+		MetaData:    issue.NewMetaData(id, "", issue.Medium, issue.High),
 		Blocklisted: blocklist,
 	}, []ast.Node{(*ast.ImportSpec)(nil)}
 }

rules/decompression_bomb.go

@@ -29,10 +29,6 @@ type decompressionBombCheck struct {
 	copyCalls   gosec.CallList
 }
 
-func (d *decompressionBombCheck) ID() string {
-	return d.MetaData.ID
-}
-
 func containsReaderCall(node ast.Node, ctx *gosec.Context, list gosec.CallList) bool {
 	if list.ContainsPkgCallExpr(node, ctx, false) != nil {
 		return true
@@ -91,28 +87,21 @@ func (d *decompressionBombCheck) Match(node ast.Node, ctx *gosec.Context) (*issu
 
 // NewDecompressionBombCheck detects potential DoS via decompression bomb
 func NewDecompressionBombCheck(id string, _ gosec.Config) (gosec.Rule, []ast.Node) {
-	readerCalls := gosec.NewCallList()
-	readerCalls.Add("compress/gzip", "NewReader")
-	readerCalls.AddAll("compress/zlib", "NewReader", "NewReaderDict")
-	readerCalls.Add("compress/bzip2", "NewReader")
-	readerCalls.AddAll("compress/flate", "NewReader", "NewReaderDict")
-	readerCalls.Add("compress/lzw", "NewReader")
-	readerCalls.Add("archive/tar", "NewReader")
-	readerCalls.Add("archive/zip", "NewReader")
-	readerCalls.Add("*archive/zip.File", "Open")
+	rule := &decompressionBombCheck{
+		MetaData:    issue.NewMetaData(id, "Potential DoS vulnerability via decompression bomb", issue.Medium, issue.Medium),
+		readerCalls: gosec.NewCallList(),
+		copyCalls:   gosec.NewCallList(),
+	}
+	rule.readerCalls.Add("compress/gzip", "NewReader")
+	rule.readerCalls.AddAll("compress/zlib", "NewReader", "NewReaderDict")
+	rule.readerCalls.Add("compress/bzip2", "NewReader")
+	rule.readerCalls.AddAll("compress/flate", "NewReader", "NewReaderDict")
+	rule.readerCalls.Add("compress/lzw", "NewReader")
+	rule.readerCalls.Add("archive/tar", "NewReader")
+	rule.readerCalls.Add("archive/zip", "NewReader")
+	rule.readerCalls.Add("*archive/zip.File", "Open")
 
-	copyCalls := gosec.NewCallList()
-	copyCalls.Add("io", "Copy")
-	copyCalls.Add("io", "CopyBuffer")
+	rule.copyCalls.AddAll("io", "Copy", "CopyBuffer")
 
-	return &decompressionBombCheck{
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.Medium,
-			What:       "Potential DoS vulnerability via decompression bomb",
-		},
-		readerCalls: readerCalls,
-		copyCalls:   copyCalls,
-	}, []ast.Node{(*ast.AssignStmt)(nil), (*ast.CallExpr)(nil)}
+	return rule, []ast.Node{(*ast.AssignStmt)(nil), (*ast.CallExpr)(nil)}
 }

rules/directory_traversal.go

@@ -13,10 +13,6 @@ type traversal struct {
 	issue.MetaData
 }
 
-func (r *traversal) ID() string {
-	return r.MetaData.ID
-}
-
 func (r *traversal) Match(n ast.Node, ctx *gosec.Context) (*issue.Issue, error) {
 	switch node := n.(type) {
 	case *ast.CallExpr:
@@ -54,12 +50,7 @@ func NewDirectoryTraversal(id string, conf gosec.Config) (gosec.Rule, []ast.Node
 	}
 
 	return &traversal{
-		pattern: regexp.MustCompile(pattern),
-		MetaData: issue.MetaData{
-			ID:         id,
-			What:       "Potential directory traversal",
-			Confidence: issue.Medium,
-			Severity:   issue.Medium,
-		},
+		pattern:  regexp.MustCompile(pattern),
+		MetaData: issue.NewMetaData(id, "Potential directory traversal", issue.Medium, issue.Medium),
 	}, []ast.Node{(*ast.CallExpr)(nil)}
 }

rules/errors.go

@@ -27,10 +27,6 @@ type noErrorCheck struct {
 	whitelist gosec.CallList
 }
 
-func (r *noErrorCheck) ID() string {
-	return r.MetaData.ID
-}
-
 func returnsError(callExpr *ast.CallExpr, ctx *gosec.Context) int {
 	if tv := ctx.Info.TypeOf(callExpr); tv != nil {
 		switch t := tv.(type) {
@@ -102,12 +98,7 @@ func NewNoErrorCheck(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	}
 
 	return &noErrorCheck{
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Low,
-			Confidence: issue.High,
-			What:       "Errors unhandled",
-		},
+		MetaData:  issue.NewMetaData(id, "Errors unhandled", issue.Low, issue.High),
 		whitelist: whitelist,
 	}, []ast.Node{(*ast.AssignStmt)(nil), (*ast.ExprStmt)(nil)}
 }

rules/fileperms.go

@@ -30,11 +30,6 @@ type filePermissions struct {
 	calls []string
 }
 
-// ID returns the ID of the rule.
-func (r *filePermissions) ID() string {
-	return r.MetaData.ID
-}
-
 func getConfiguredMode(conf map[string]interface{}, configKey string, defaultMode int64) int64 {
 	mode := defaultMode
 	if value, ok := conf[configKey]; ok {
@@ -85,15 +80,10 @@ func isOsPerm(n ast.Node) bool {
 func NewWritePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	mode := getConfiguredMode(conf, id, 0o600)
 	return &filePermissions{
-		mode:  mode,
-		pkgs:  []string{"io/ioutil", "os"},
-		calls: []string{"WriteFile"},
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-			What:       fmt.Sprintf("Expect WriteFile permissions to be %#o or less", mode),
-		},
+		mode:     mode,
+		pkgs:     []string{"io/ioutil", "os"},
+		calls:    []string{"WriteFile"},
+		MetaData: issue.NewMetaData(id, fmt.Sprintf("Expect WriteFile permissions to be %#o or less", mode), issue.Medium, issue.High),
 	}, []ast.Node{(*ast.CallExpr)(nil)}
 }
 
@@ -102,15 +92,10 @@ func NewWritePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 func NewFilePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	mode := getConfiguredMode(conf, id, 0o600)
 	return &filePermissions{
-		mode:  mode,
-		pkgs:  []string{"os"},
-		calls: []string{"OpenFile", "Chmod"},
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-			What:       fmt.Sprintf("Expect file permissions to be %#o or less", mode),
-		},
+		mode:     mode,
+		pkgs:     []string{"os"},
+		calls:    []string{"OpenFile", "Chmod"},
+		MetaData: issue.NewMetaData(id, fmt.Sprintf("Expect file permissions to be %#o or less", mode), issue.Medium, issue.High),
 	}, []ast.Node{(*ast.CallExpr)(nil)}
 }
 
@@ -119,15 +104,10 @@ func NewFilePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 func NewMkdirPerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	mode := getConfiguredMode(conf, id, 0o750)
 	return &filePermissions{
-		mode:  mode,
-		pkgs:  []string{"os"},
-		calls: []string{"Mkdir", "MkdirAll"},
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-			What:       fmt.Sprintf("Expect directory permissions to be %#o or less", mode),
-		},
+		mode:     mode,
+		pkgs:     []string{"os"},
+		calls:    []string{"Mkdir", "MkdirAll"},
+		MetaData: issue.NewMetaData(id, fmt.Sprintf("Expect directory permissions to be %#o or less", mode), issue.Medium, issue.High),
 	}, []ast.Node{(*ast.CallExpr)(nil)}
 }
 
@@ -140,11 +120,6 @@ type osCreatePermissions struct {
 
 const defaultOsCreateMode = 0o666
 
-// ID returns the ID of the rule.
-func (r *osCreatePermissions) ID() string {
-	return r.MetaData.ID
-}
-
 // Match checks if the rule is matched.
 func (r *osCreatePermissions) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) {
 	for _, pkg := range r.pkgs {
@@ -165,12 +140,7 @@ func NewOsCreatePerms(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 		mode:  mode,
 		pkgs:  []string{"os"},
 		calls: []string{"Create"},
-		MetaData: issue.MetaData{
-			ID:         id,
-			Severity:   issue.Medium,
-			Confidence: issue.High,
-			What: fmt.Sprintf("Expect file permissions to be %#o or less but os.Create used with default permissions %#o",
-				mode, defaultOsCreateMode),
-		},
+		MetaData: issue.NewMetaData(id, fmt.Sprintf("Expect file permissions to be %#o or less but os.Create used with default permissions %#o",
+			mode, defaultOsCreateMode), issue.Medium, issue.High),
 	}, []ast.Node{(*ast.CallExpr)(nil)}
 }