From 91132985a7ff76db390949ac765113cfd3178fa7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Aug 2023 02:02:22 +0000 Subject: mod: do: bump github.com/golangci/golangci-lint from 1.54.1 to 1.54.2 Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.54.1 to 1.54.2. - [Release notes](https://github.com/golangci/golangci-lint/releases) - [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md) - [Commits](https://github.com/golangci/golangci-lint/compare/v1.54.1...v1.54.2) --- updated-dependencies: - dependency-name: github.com/golangci/golangci-lint dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- vendor/github.com/securego/gosec/v2/.golangci.yml | 1 - vendor/github.com/securego/gosec/v2/README.md | 1 + vendor/github.com/securego/gosec/v2/action.yml | 2 +- vendor/github.com/securego/gosec/v2/analyzer.go | 6 +- .../github.com/securego/gosec/v2/analyzers/ssrf.go | 2 +- .../github.com/securego/gosec/v2/analyzers/util.go | 4 +- vendor/github.com/securego/gosec/v2/config.go | 5 + vendor/github.com/securego/gosec/v2/helpers.go | 56 ++- vendor/github.com/securego/gosec/v2/issue/issue.go | 1 + vendor/github.com/securego/gosec/v2/rule.go | 2 +- .../gosec/v2/rules/hardcoded_credentials.go | 57 ++- .../securego/gosec/v2/rules/implicit_aliasing.go | 24 +- .../github.com/securego/gosec/v2/rules/rulelist.go | 1 + .../securego/gosec/v2/rules/slice_bounds.go | 405 +++++++++++++++++++++ vendor/github.com/securego/gosec/v2/rules/sql.go | 51 ++- .../github.com/securego/gosec/v2/rules/subproc.go | 2 +- 16 files changed, 602 insertions(+), 18 deletions(-) create mode 100644 vendor/github.com/securego/gosec/v2/rules/slice_bounds.go (limited to 'vendor/github.com/securego') diff --git a/vendor/github.com/securego/gosec/v2/.golangci.yml b/vendor/github.com/securego/gosec/v2/.golangci.yml index b12140a25..d6c5de7ba 100644 --- a/vendor/github.com/securego/gosec/v2/.golangci.yml +++ b/vendor/github.com/securego/gosec/v2/.golangci.yml @@ -2,7 +2,6 @@ linters: enable: - asciicheck - bodyclose - - depguard - dogsled - durationcheck - errcheck diff --git a/vendor/github.com/securego/gosec/v2/README.md b/vendor/github.com/securego/gosec/v2/README.md index 71e032d80..6c6d2982c 100644 --- a/vendor/github.com/securego/gosec/v2/README.md +++ b/vendor/github.com/securego/gosec/v2/README.md @@ -167,6 +167,7 @@ directory you can supply `./...` as the input argument. - G504: Import blocklist: net/http/cgi - G505: Import blocklist: crypto/sha1 - G601: Implicit memory aliasing of items from a range statement +- G602: Slice access out of bounds ### Retired rules diff --git a/vendor/github.com/securego/gosec/v2/action.yml b/vendor/github.com/securego/gosec/v2/action.yml index 0320f0c21..8e28c346d 100644 --- a/vendor/github.com/securego/gosec/v2/action.yml +++ b/vendor/github.com/securego/gosec/v2/action.yml @@ -10,7 +10,7 @@ inputs: runs: using: 'docker' - image: 'docker://securego/gosec:2.15.0' + image: 'docker://securego/gosec:2.16.0' args: - ${{ inputs.args }} diff --git a/vendor/github.com/securego/gosec/v2/analyzer.go b/vendor/github.com/securego/gosec/v2/analyzer.go index 830d338e4..023514b8a 100644 --- a/vendor/github.com/securego/gosec/v2/analyzer.go +++ b/vendor/github.com/securego/gosec/v2/analyzer.go @@ -59,7 +59,7 @@ var generatedCodePattern = regexp.MustCompile(`^// Code generated .* DO NOT EDIT // The Context is populated with data parsed from the source code as it is scanned. // It is passed through to all rule functions as they are called. Rules may use -// this data in conjunction withe the encountered AST node. +// this data in conjunction with the encountered AST node. type Context struct { FileSet *token.FileSet Comments ast.CommentMap @@ -449,10 +449,12 @@ func (gosec *Analyzer) ignore(n ast.Node) map[string]issue.SuppressionInfo { if groups, ok := gosec.context.Comments[n]; ok && !gosec.ignoreNosec { // Checks if an alternative for #nosec is set and, if not, uses the default. - noSecDefaultTag := "#nosec" + noSecDefaultTag := NoSecTag(string(Nosec)) noSecAlternativeTag, err := gosec.config.GetGlobal(NoSecAlternative) if err != nil { noSecAlternativeTag = noSecDefaultTag + } else { + noSecAlternativeTag = NoSecTag(noSecAlternativeTag) } for _, group := range groups { diff --git a/vendor/github.com/securego/gosec/v2/analyzers/ssrf.go b/vendor/github.com/securego/gosec/v2/analyzers/ssrf.go index a9dbd9500..70e0211f1 100644 --- a/vendor/github.com/securego/gosec/v2/analyzers/ssrf.go +++ b/vendor/github.com/securego/gosec/v2/analyzers/ssrf.go @@ -46,7 +46,7 @@ func runSSRF(pass *analysis.Pass) (interface{}, error) { if callee != nil { ssaResult.Logger.Printf("callee: %s\n", callee) return newIssue(pass.Analyzer.Name, - "not implemeted", + "not implemented", pass.Fset, instr.Call.Pos(), issue.Low, issue.High), nil } } diff --git a/vendor/github.com/securego/gosec/v2/analyzers/util.go b/vendor/github.com/securego/gosec/v2/analyzers/util.go index b090a3e45..f1bd867ae 100644 --- a/vendor/github.com/securego/gosec/v2/analyzers/util.go +++ b/vendor/github.com/securego/gosec/v2/analyzers/util.go @@ -28,7 +28,7 @@ import ( ) // SSAAnalyzerResult contains various information returned by the -// SSA analysis along with some configuraion +// SSA analysis along with some configuration type SSAAnalyzerResult struct { Config map[string]interface{} Logger *log.Logger @@ -42,7 +42,7 @@ func BuildDefaultAnalyzers() []*analysis.Analyzer { } } -// getSSAResult retrives the SSA result from analysis pass +// getSSAResult retrieves the SSA result from analysis pass func getSSAResult(pass *analysis.Pass) (*SSAAnalyzerResult, error) { result, ok := pass.ResultOf[buildssa.Analyzer] if !ok { diff --git a/vendor/github.com/securego/gosec/v2/config.go b/vendor/github.com/securego/gosec/v2/config.go index ca4cf2175..9cbb7a713 100644 --- a/vendor/github.com/securego/gosec/v2/config.go +++ b/vendor/github.com/securego/gosec/v2/config.go @@ -33,6 +33,11 @@ const ( SSA GlobalOption = "ssa" ) +// NoSecTag returns the tag used to disable gosec for a line of code. +func NoSecTag(tag string) string { + return fmt.Sprintf("%s%s", "#", tag) +} + // Config is used to provide configuration and customization to each of the rules. type Config map[string]interface{} diff --git a/vendor/github.com/securego/gosec/v2/helpers.go b/vendor/github.com/securego/gosec/v2/helpers.go index 08b7893eb..b4c23e5bb 100644 --- a/vendor/github.com/securego/gosec/v2/helpers.go +++ b/vendor/github.com/securego/gosec/v2/helpers.go @@ -96,11 +96,46 @@ func GetChar(n ast.Node) (byte, error) { return 0, fmt.Errorf("Unexpected AST node type: %T", n) } +// GetStringRecursive will recursively walk down a tree of *ast.BinaryExpr. It will then concat the results, and return. +// Unlike the other getters, it does _not_ raise an error for unknown ast.Node types. At the base, the recursion will hit a non-BinaryExpr type, +// either BasicLit or other, so it's not an error case. It will only error if `strconv.Unquote` errors. This matters, because there's +// currently functionality that relies on error values being returned by GetString if and when it hits a non-basiclit string node type, +// hence for cases where recursion is needed, we use this separate function, so that we can still be backwards compatbile. +// +// This was added to handle a SQL injection concatenation case where the injected value is infixed between two strings, not at the start or end. See example below +// +// Do note that this will omit non-string values. So for example, if you were to use this node: +// ```go +// q := "SELECT * FROM foo WHERE name = '" + os.Args[0] + "' AND 1=1" // will result in "SELECT * FROM foo WHERE ” AND 1=1" + +func GetStringRecursive(n ast.Node) (string, error) { + if node, ok := n.(*ast.BasicLit); ok && node.Kind == token.STRING { + return strconv.Unquote(node.Value) + } + + if expr, ok := n.(*ast.BinaryExpr); ok { + x, err := GetStringRecursive(expr.X) + if err != nil { + return "", err + } + + y, err := GetStringRecursive(expr.Y) + if err != nil { + return "", err + } + + return x + y, nil + } + + return "", nil +} + // GetString will read and return a string value from an ast.BasicLit func GetString(n ast.Node) (string, error) { if node, ok := n.(*ast.BasicLit); ok && node.Kind == token.STRING { return strconv.Unquote(node.Value) } + return "", fmt.Errorf("Unexpected AST node type: %T", n) } @@ -201,22 +236,21 @@ func GetCallStringArgsValues(n ast.Node, _ *Context) []string { return values } -// GetIdentStringValues return the string values of an Ident if they can be resolved -func GetIdentStringValues(ident *ast.Ident) []string { +func getIdentStringValues(ident *ast.Ident, stringFinder func(ast.Node) (string, error)) []string { values := []string{} obj := ident.Obj if obj != nil { switch decl := obj.Decl.(type) { case *ast.ValueSpec: for _, v := range decl.Values { - value, err := GetString(v) + value, err := stringFinder(v) if err == nil { values = append(values, value) } } case *ast.AssignStmt: for _, v := range decl.Rhs { - value, err := GetString(v) + value, err := stringFinder(v) if err == nil { values = append(values, value) } @@ -226,6 +260,18 @@ func GetIdentStringValues(ident *ast.Ident) []string { return values } +// getIdentStringRecursive returns the string of values of an Ident if they can be resolved +// The difference between this and GetIdentStringValues is that it will attempt to resolve the strings recursively, +// if it is passed a *ast.BinaryExpr. See GetStringRecursive for details +func GetIdentStringValuesRecursive(ident *ast.Ident) []string { + return getIdentStringValues(ident, GetStringRecursive) +} + +// GetIdentStringValues return the string values of an Ident if they can be resolved +func GetIdentStringValues(ident *ast.Ident) []string { + return getIdentStringValues(ident, GetString) +} + // GetBinaryExprOperands returns all operands of a binary expression by traversing // the expression tree func GetBinaryExprOperands(be *ast.BinaryExpr) []ast.Node { @@ -301,7 +347,7 @@ func Getenv(key, userDefault string) string { return userDefault } -// GetPkgRelativePath returns the Go relative relative path derived +// GetPkgRelativePath returns the Go relative path derived // form the given path func GetPkgRelativePath(path string) (string, error) { abspath, err := filepath.Abs(path) diff --git a/vendor/github.com/securego/gosec/v2/issue/issue.go b/vendor/github.com/securego/gosec/v2/issue/issue.go index 5bf00dec2..db4d630fa 100644 --- a/vendor/github.com/securego/gosec/v2/issue/issue.go +++ b/vendor/github.com/securego/gosec/v2/issue/issue.go @@ -87,6 +87,7 @@ var ruleToCWE = map[string]string{ "G504": "327", "G505": "327", "G601": "118", + "G602": "118", } // Issue is returned by a gosec rule if it discovers an issue with the scanned code. diff --git a/vendor/github.com/securego/gosec/v2/rule.go b/vendor/github.com/securego/gosec/v2/rule.go index 5e973b6ac..490a25da0 100644 --- a/vendor/github.com/securego/gosec/v2/rule.go +++ b/vendor/github.com/securego/gosec/v2/rule.go @@ -43,7 +43,7 @@ func NewRuleSet() RuleSet { return RuleSet{make(map[reflect.Type][]Rule), make(map[string]bool)} } -// Register adds a trigger for the supplied rule for the the +// Register adds a trigger for the supplied rule for the // specified ast nodes. func (r RuleSet) Register(rule Rule, isSuppressed bool, nodes ...ast.Node) { for _, n := range nodes { diff --git a/vendor/github.com/securego/gosec/v2/rules/hardcoded_credentials.go b/vendor/github.com/securego/gosec/v2/rules/hardcoded_credentials.go index eac50d7c9..ea8386084 100644 --- a/vendor/github.com/securego/gosec/v2/rules/hardcoded_credentials.go +++ b/vendor/github.com/securego/gosec/v2/rules/hardcoded_credentials.go @@ -20,7 +20,7 @@ import ( "regexp" "strconv" - zxcvbn "github.com/nbutton23/zxcvbn-go" + zxcvbn "github.com/ccojocar/zxcvbn-go" "github.com/securego/gosec/v2" "github.com/securego/gosec/v2/issue" @@ -29,6 +29,7 @@ import ( type credentials struct { issue.MetaData pattern *regexp.Regexp + patternValue *regexp.Regexp // Pattern for matching string values (LHS on assign statements) entropyThreshold float64 perCharThreshold float64 truncate int @@ -70,6 +71,7 @@ func (r *credentials) Match(n ast.Node, ctx *gosec.Context) (*issue.Issue, error func (r *credentials) matchAssign(assign *ast.AssignStmt, ctx *gosec.Context) (*issue.Issue, error) { for _, i := range assign.Lhs { if ident, ok := i.(*ast.Ident); ok { + // First check LHS to find anything being assigned to variables whose name appears to be a cred if r.pattern.MatchString(ident.Name) { for _, e := range assign.Rhs { if val, err := gosec.GetString(e); err == nil { @@ -79,12 +81,28 @@ func (r *credentials) matchAssign(assign *ast.AssignStmt, ctx *gosec.Context) (* } } } + + // Now that no names were matched, match the RHS to see if the actual values being assigned are creds + for _, e := range assign.Rhs { + val, err := gosec.GetString(e) + if err != nil { + continue + } + + if r.patternValue.MatchString(val) { + if r.ignoreEntropy || r.isHighEntropyString(val) { + return ctx.NewIssue(assign, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + } } } return nil, nil } func (r *credentials) matchValueSpec(valueSpec *ast.ValueSpec, ctx *gosec.Context) (*issue.Issue, error) { + // Running match against the variable name(s) first. Will catch any creds whose var name matches the pattern, + // then will go back over to check the values themselves. for index, ident := range valueSpec.Names { if r.pattern.MatchString(ident.Name) && valueSpec.Values != nil { // const foo, bar = "same value" @@ -98,6 +116,18 @@ func (r *credentials) matchValueSpec(valueSpec *ast.ValueSpec, ctx *gosec.Contex } } } + + // Now that no variable names have been matched, match the actual values to find any creds + for _, ident := range valueSpec.Values { + if val, err := gosec.GetString(ident); err == nil { + if r.patternValue.MatchString(val) { + if r.ignoreEntropy || r.isHighEntropyString(val) { + return ctx.NewIssue(valueSpec, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + } + } + return nil, nil } @@ -119,6 +149,22 @@ func (r *credentials) matchEqualityCheck(binaryExpr *ast.BinaryExpr, ctx *gosec. } } } + + // Now that the variable names have been checked, and no matches were found, make sure that + // either the left or right operands is a string literal so we can match the value. + identStrConst, ok := binaryExpr.X.(*ast.BasicLit) + if !ok { + identStrConst, ok = binaryExpr.Y.(*ast.BasicLit) + } + + if ok && identStrConst.Kind == token.STRING { + s, _ := gosec.GetString(identStrConst) + if r.patternValue.MatchString(s) { + if r.ignoreEntropy || r.isHighEntropyString(s) { + return ctx.NewIssue(binaryExpr, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + } } return nil, nil } @@ -127,6 +173,7 @@ func (r *credentials) matchEqualityCheck(binaryExpr *ast.BinaryExpr, ctx *gosec. // assigned to variables that appear to be related to credentials. func NewHardcodedCredentials(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { pattern := `(?i)passwd|pass|password|pwd|secret|token|pw|apiKey|bearer|cred` + patternValue := "(?i)(^(.*[:;,](\\s)*)?[a-f0-9]{64}$)|(AIza[0-9A-Za-z-_]{35})|(^(.*[:;,](\\s)*)?github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}$)|(^(.*[:;,](\\s)*)?[0-9a-zA-Z-_]{24}$)" entropyThreshold := 80.0 perCharThreshold := 3.0 ignoreEntropy := false @@ -138,6 +185,13 @@ func NewHardcodedCredentials(id string, conf gosec.Config) (gosec.Rule, []ast.No pattern = cfgPattern } } + + if configPatternValue, ok := conf["patternValue"]; ok { + if cfgPatternValue, ok := configPatternValue.(string); ok { + patternValue = cfgPatternValue + } + } + if configIgnoreEntropy, ok := conf["ignore_entropy"]; ok { if cfgIgnoreEntropy, ok := configIgnoreEntropy.(bool); ok { ignoreEntropy = cfgIgnoreEntropy @@ -168,6 +222,7 @@ func NewHardcodedCredentials(id string, conf gosec.Config) (gosec.Rule, []ast.No return &credentials{ pattern: regexp.MustCompile(pattern), + patternValue: regexp.MustCompile(patternValue), entropyThreshold: entropyThreshold, perCharThreshold: perCharThreshold, ignoreEntropy: ignoreEntropy, diff --git a/vendor/github.com/securego/gosec/v2/rules/implicit_aliasing.go b/vendor/github.com/securego/gosec/v2/rules/implicit_aliasing.go index 70678e29a..32e2fd205 100644 --- a/vendor/github.com/securego/gosec/v2/rules/implicit_aliasing.go +++ b/vendor/github.com/securego/gosec/v2/rules/implicit_aliasing.go @@ -28,6 +28,26 @@ func containsUnary(exprs []*ast.UnaryExpr, expr *ast.UnaryExpr) bool { return false } +func getIdentExpr(expr ast.Expr) *ast.Ident { + switch node := expr.(type) { + case *ast.Ident: + return node + case *ast.SelectorExpr: + return getIdentExpr(node.X) + case *ast.UnaryExpr: + switch e := node.X.(type) { + case *ast.Ident: + return e + case *ast.SelectorExpr: + return getIdentExpr(e.X) + default: + return nil + } + default: + return nil + } +} + func (r *implicitAliasing) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) { switch node := n.(type) { case *ast.RangeStmt: @@ -72,8 +92,8 @@ 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 ident, ok := node.X.(*ast.Ident); ok && node.Op.String() == "&" { - if _, contains := r.aliases[ident.Obj]; contains { + if identExpr := 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 } } diff --git a/vendor/github.com/securego/gosec/v2/rules/rulelist.go b/vendor/github.com/securego/gosec/v2/rules/rulelist.go index d856eccad..316691f61 100644 --- a/vendor/github.com/securego/gosec/v2/rules/rulelist.go +++ b/vendor/github.com/securego/gosec/v2/rules/rulelist.go @@ -107,6 +107,7 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList { // memory safety {"G601", "Implicit memory aliasing in RangeStmt", NewImplicitAliasing}, + {"G602", "Slice access out of bounds", NewSliceBoundCheck}, } ruleMap := make(map[string]RuleDefinition) diff --git a/vendor/github.com/securego/gosec/v2/rules/slice_bounds.go b/vendor/github.com/securego/gosec/v2/rules/slice_bounds.go new file mode 100644 index 000000000..04811bb50 --- /dev/null +++ b/vendor/github.com/securego/gosec/v2/rules/slice_bounds.go @@ -0,0 +1,405 @@ +package rules + +import ( + "fmt" + "go/ast" + "go/types" + + "github.com/securego/gosec/v2" + "github.com/securego/gosec/v2/issue" +) + +// sliceOutOfBounds is a rule which checks for slices which are accessed outside their capacity, +// either through indexing it out of bounds or through slice expressions whose low or high index +// are out of bounds. +type sliceOutOfBounds struct { + sliceCaps map[*ast.CallExpr]map[string]*int64 // Capacities of slices. Maps function call -> var name -> value. + currentScope *types.Scope // Current scope. Map is cleared when scope changes. + currentFuncName string // Current function. + funcCallArgs map[string][]*int64 // Caps to load once a func declaration is scanned. + issue.MetaData // Metadata for this rule. +} + +// ID returns the rule ID for sliceOutOfBounds: G602. +func (s *sliceOutOfBounds) ID() string { + return s.MetaData.ID +} + +func (s *sliceOutOfBounds) Match(node ast.Node, ctx *gosec.Context) (*issue.Issue, error) { + if s.currentScope == nil { + s.currentScope = ctx.Pkg.Scope() + } else if s.currentScope != ctx.Pkg.Scope() { + s.currentScope = ctx.Pkg.Scope() + + // Clear slice map, since we are in a new scope + sliceMapNil := make(map[string]*int64) + sliceCaps := make(map[*ast.CallExpr]map[string]*int64) + sliceCaps[nil] = sliceMapNil + s.sliceCaps = sliceCaps + } + + switch node := node.(type) { + case *ast.AssignStmt: + return s.matchAssign(node, ctx) + case *ast.SliceExpr: + return s.matchSliceExpr(node, ctx) + case *ast.IndexExpr: + return s.matchIndexExpr(node, ctx) + case *ast.FuncDecl: + s.currentFuncName = node.Name.Name + s.loadArgCaps(node) + case *ast.CallExpr: + if _, ok := node.Fun.(*ast.FuncLit); ok { + // Do nothing with func literals for now. + break + } + + sliceMap := make(map[string]*int64) + s.sliceCaps[node] = sliceMap + s.setupCallArgCaps(node, ctx) + } + return nil, nil +} + +// updateSliceCaps takes in a variable name and a map of calls we are updating the variables for to the updated values +// and will add it to the sliceCaps map. +func (s *sliceOutOfBounds) updateSliceCaps(varName string, caps map[*ast.CallExpr]*int64) { + for callExpr, cap := range caps { + s.sliceCaps[callExpr][varName] = cap + } +} + +// getAllCalls returns all CallExprs that are calls to the given function. +func (s *sliceOutOfBounds) getAllCalls(funcName string, ctx *gosec.Context) []*ast.CallExpr { + calls := []*ast.CallExpr{} + + for callExpr := range s.sliceCaps { + if callExpr != nil { + // Compare the names of the function the code is scanning with the current call we are iterating over + _, callFuncName, err := gosec.GetCallInfo(callExpr, ctx) + if err != nil { + continue + } + + if callFuncName == funcName { + calls = append(calls, callExpr) + } + } + } + return calls +} + +// getSliceCapsForFunc gets all the capacities for slice with given name that are stored for each call to the passed function. +func (s *sliceOutOfBounds) getSliceCapsForFunc(funcName string, varName string, ctx *gosec.Context) map[*ast.CallExpr]*int64 { + caps := make(map[*ast.CallExpr]*int64) + + calls := s.getAllCalls(funcName, ctx) + for _, call := range calls { + if callCaps, ok := s.sliceCaps[call]; ok { + caps[call] = callCaps[varName] + } + } + + return caps +} + +// setupCallArgCaps evaluates and saves the caps for any slices in the args so they can be validated when the function is scanned. +func (s *sliceOutOfBounds) setupCallArgCaps(callExpr *ast.CallExpr, ctx *gosec.Context) { + // Array of caps to be loaded once the function declaration is scanned + funcCallArgs := []*int64{} + + // Get function name + _, funcName, err := gosec.GetCallInfo(callExpr, ctx) + if err != nil { + return + } + + for _, arg := range callExpr.Args { + switch node := arg.(type) { + case *ast.SliceExpr: + caps := s.evaluateSliceExpr(node, ctx) + + // Simplifying assumption: use the lowest capacity. Storing all possible capacities for slices passed + // to a function call would catch the most issues, but would require a data structure like a stack and a + // reworking of the code for scanning itself. Use the lowest capacity, as this would be more likely to + // raise an issue for being out of bounds. + var lowestCap *int64 + for _, cap := range caps { + if cap == nil { + continue + } + + if lowestCap == nil { + lowestCap = cap + } else if *lowestCap > *cap { + lowestCap = cap + } + } + + if lowestCap == nil { + funcCallArgs = append(funcCallArgs, nil) + continue + } + + // Now create a map of just this value to add it to the sliceCaps + funcCallArgs = append(funcCallArgs, lowestCap) + case *ast.Ident: + ident := arg.(*ast.Ident) + caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx) + + var lowestCap *int64 + for _, cap := range caps { + if cap == nil { + continue + } + + if lowestCap == nil { + lowestCap = cap + } else if *lowestCap > *cap { + lowestCap = cap + } + } + + if lowestCap == nil { + funcCallArgs = append(funcCallArgs, nil) + continue + } + + // Now create a map of just this value to add it to the sliceCaps + funcCallArgs = append(funcCallArgs, lowestCap) + default: + funcCallArgs = append(funcCallArgs, nil) + } + } + s.funcCallArgs[funcName] = funcCallArgs +} + +// loadArgCaps loads caps that were saved for a call to this function. +func (s *sliceOutOfBounds) loadArgCaps(funcDecl *ast.FuncDecl) { + sliceMap := make(map[string]*int64) + funcName := funcDecl.Name.Name + + // Create a dummmy call expr for the new function. This is so we can still store args for + // functions which are not explicitly called in the code by other functions (specifically, main). + ident := ast.NewIdent(funcName) + dummyCallExpr := ast.CallExpr{ + Fun: ident, + } + + argCaps, ok := s.funcCallArgs[funcName] + if !ok || len(argCaps) == 0 { + s.sliceCaps[&dummyCallExpr] = sliceMap + return + } + + params := funcDecl.Type.Params.List + if len(params) > len(argCaps) { + return // Length of params and args doesn't match, so don't do anything with this. + } + + for it := range params { + capacity := argCaps[it] + if capacity == nil { + continue + } + + if len(params[it].Names) == 0 { + continue + } + + if paramName := params[it].Names[0]; paramName != nil { + sliceMap[paramName.Name] = capacity + } + } + + s.sliceCaps[&dummyCallExpr] = sliceMap +} + +// matchSliceMake matches calls to make() and stores the capacity of the new slice in the map to compare against future slice usage. +func (s *sliceOutOfBounds) matchSliceMake(funcCall *ast.CallExpr, sliceName string, ctx *gosec.Context) (*issue.Issue, error) { + _, funcName, err := gosec.GetCallInfo(funcCall, ctx) + if err != nil || funcName != "make" { + return nil, nil + } + + var capacityArg int + if len(funcCall.Args) < 2 { + return nil, nil // No size passed + } else if len(funcCall.Args) == 2 { + capacityArg = 1 + } else if len(funcCall.Args) == 3 { + capacityArg = 2 + } else { + return nil, nil // Unexpected, args should always be 2 or 3 + } + + // Check and get the capacity of the slice passed to make. It must be a literal value, since we aren't evaluating the expression. + sliceCapLit, ok := funcCall.Args[capacityArg].(*ast.BasicLit) + if !ok { + return nil, nil + } + + capacity, err := gosec.GetInt(sliceCapLit) + if err != nil { + return nil, nil + } + + caps := s.getSliceCapsForFunc(s.currentFuncName, sliceName, ctx) + for callExpr := range caps { + caps[callExpr] = &capacity + } + + s.updateSliceCaps(sliceName, caps) + return nil, nil +} + +// evaluateSliceExpr takes a slice expression and evaluates what the capacity of said slice is for each of the +// calls to the current function. Returns map of the call expressions of each call to the current function to +// the evaluated capacities. +func (s *sliceOutOfBounds) evaluateSliceExpr(node *ast.SliceExpr, ctx *gosec.Context) map[*ast.CallExpr]*int64 { + // Get ident to get name + ident, ok := node.X.(*ast.Ident) + if !ok { + return nil + } + + // Get cap of old slice to calculate this new slice's cap + caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx) + for callExpr, oldCap := range caps { + if oldCap == nil { + continue + } + + // Get and check low value + lowIdent, ok := node.Low.(*ast.BasicLit) + if ok && lowIdent != nil { + low, _ := gosec.GetInt(lowIdent) + + newCap := *oldCap - low + caps[callExpr] = &newCap + } else if lowIdent == nil { // If no lower bound, capacity will be same + continue + } + } + + return caps +} + +// matchSliceAssignment matches slice assignments, calculates capacity of slice if possible to store it in map. +func (s *sliceOutOfBounds) matchSliceAssignment(node *ast.SliceExpr, sliceName string, ctx *gosec.Context) (*issue.Issue, error) { + // First do the normal match that verifies the slice expr is not out of bounds + if i, err := s.matchSliceExpr(node, ctx); err != nil { + return i, fmt.Errorf("There was an error while matching a slice expression to check slice bounds for %s: %w", sliceName, err) + } + + // Now that the assignment is (presumably) successfully, we can calculate the capacity and add this new slice to the map + caps := s.evaluateSliceExpr(node, ctx) + s.updateSliceCaps(sliceName, caps) + + return nil, nil +} + +// matchAssign matches checks if an assignment statement is making a slice, or if it is assigning a slice. +func (s *sliceOutOfBounds) matchAssign(node *ast.AssignStmt, ctx *gosec.Context) (*issue.Issue, error) { + // Check RHS for calls to make() so we can get the actual size of the slice + for it, i := range node.Rhs { + // Get the slice name so we can associate the cap with the slice in the map + sliceIdent, ok := node.Lhs[it].(*ast.Ident) + if !ok { + return nil, nil + } + sliceName := sliceIdent.Name + + switch expr := i.(type) { + case *ast.CallExpr: // Check for and handle call to make() + return s.matchSliceMake(expr, sliceName, ctx) + case *ast.SliceExpr: // Handle assignments to a slice + return s.matchSliceAssignment(expr, sliceName, ctx) + } + } + return nil, nil +} + +// matchSliceExpr validates that a given slice expression (eg, slice[10:30]) is not out of bounds. +func (s *sliceOutOfBounds) matchSliceExpr(node *ast.SliceExpr, ctx *gosec.Context) (*issue.Issue, error) { + // First get the slice name so we can check the size in our map + ident, ok := node.X.(*ast.Ident) + if !ok { + return nil, nil + } + + // Get slice cap from the map to compare it against high and low + caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx) + + for _, cap := range caps { + if cap == nil { + continue + } + + // Get and check high value + highIdent, ok := node.High.(*ast.BasicLit) + if ok && highIdent != nil { + high, _ := gosec.GetInt(highIdent) + if high > *cap { + return ctx.NewIssue(node, s.ID(), s.What, s.Severity, s.Confidence), nil + } + } + + // Get and check low value + lowIdent, ok := node.Low.(*ast.BasicLit) + if ok && lowIdent != nil { + low, _ := gosec.GetInt(lowIdent) + if low > *cap { + return ctx.NewIssue(node, s.ID(), s.What, s.Severity, s.Confidence), nil + } + } + } + + return nil, nil +} + +// matchIndexExpr validates that an index into a slice is not out of bounds. +func (s *sliceOutOfBounds) matchIndexExpr(node *ast.IndexExpr, ctx *gosec.Context) (*issue.Issue, error) { + // First get the slice name so we can check the size in our map + ident, ok := node.X.(*ast.Ident) + if !ok { + return nil, nil + } + + // Get slice cap from the map to compare it against high and low + caps := s.getSliceCapsForFunc(s.currentFuncName, ident.Name, ctx) + + for _, cap := range caps { + if cap == nil { + continue + } + // Get the index literal + indexIdent, ok := node.Index.(*ast.BasicLit) + if ok && indexIdent != nil { + index, _ := gosec.GetInt(indexIdent) + if index >= *cap { + return ctx.NewIssue(node, s.ID(), s.What, s.Severity, s.Confidence), nil + } + } + } + + return nil, nil +} + +// NewSliceBoundCheck attempts to find any slices being accessed out of bounds +// by reslicing or by being indexed. +func NewSliceBoundCheck(id string, _ gosec.Config) (gosec.Rule, []ast.Node) { + sliceMap := make(map[*ast.CallExpr]map[string]*int64) + + return &sliceOutOfBounds{ + sliceCaps: sliceMap, + currentFuncName: "", + funcCallArgs: make(map[string][]*int64), + MetaData: issue.MetaData{ + ID: id, + Severity: issue.Medium, + Confidence: issue.Medium, + What: "Potentially accessing slice out of bounds", + }, + }, []ast.Node{(*ast.CallExpr)(nil), (*ast.FuncDecl)(nil), (*ast.AssignStmt)(nil), (*ast.SliceExpr)(nil), (*ast.IndexExpr)(nil)} +} diff --git a/vendor/github.com/securego/gosec/v2/rules/sql.go b/vendor/github.com/securego/gosec/v2/rules/sql.go index 4085b5d26..61222bfdb 100644 --- a/vendor/github.com/securego/gosec/v2/rules/sql.go +++ b/vendor/github.com/securego/gosec/v2/rules/sql.go @@ -98,6 +98,32 @@ func (s *sqlStrConcat) ID() string { return s.MetaData.ID } +// findInjectionInBranch walks diwb a set if expressions, and will create new issues if it finds SQL injections +// This method assumes you've already verified that the branch contains SQL syntax +func (s *sqlStrConcat) findInjectionInBranch(ctx *gosec.Context, branch []ast.Expr) *ast.BinaryExpr { + for _, node := range branch { + be, ok := node.(*ast.BinaryExpr) + if !ok { + continue + } + + operands := gosec.GetBinaryExprOperands(be) + + for _, op := range operands { + if _, ok := op.(*ast.BasicLit); ok { + continue + } + + if ident, ok := op.(*ast.Ident); ok && s.checkObject(ident, ctx) { + continue + } + + return be + } + } + return nil +} + // see if we can figure out what it is func (s *sqlStrConcat) checkObject(n *ast.Ident, c *gosec.Context) bool { if n.Obj != nil { @@ -140,6 +166,28 @@ func (s *sqlStrConcat) checkQuery(call *ast.CallExpr, ctx *gosec.Context) (*issu } } + // Handle the case where an injection occurs as an infixed string concatenation, ie "SELECT * FROM foo WHERE name = '" + os.Args[0] + "' AND 1=1" + if id, ok := query.(*ast.Ident); ok { + var match bool + for _, str := range gosec.GetIdentStringValuesRecursive(id) { + if s.MatchPatterns(str) { + match = true + break + } + } + + if !match { + return nil, nil + } + + switch decl := id.Obj.Decl.(type) { + case *ast.AssignStmt: + if injection := s.findInjectionInBranch(ctx, decl.Rhs); injection != nil { + return ctx.NewIssue(injection, s.ID(), s.What, s.Severity, s.Confidence), nil + } + } + } + return nil, nil } @@ -157,6 +205,7 @@ func (s *sqlStrConcat) Match(n ast.Node, ctx *gosec.Context) (*issue.Issue, erro return s.checkQuery(sqlQueryCall, ctx) } } + return nil, nil } @@ -165,7 +214,7 @@ func NewSQLStrConcat(id string, _ gosec.Config) (gosec.Rule, []ast.Node) { rule := &sqlStrConcat{ sqlStatement: sqlStatement{ patterns: []*regexp.Regexp{ - regexp.MustCompile(`(?i)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), + regexp.MustCompile("(?i)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE)( |\n|\r|\t)"), }, MetaData: issue.MetaData{ ID: id, diff --git a/vendor/github.com/securego/gosec/v2/rules/subproc.go b/vendor/github.com/securego/gosec/v2/rules/subproc.go index ea50d692d..1e2cedaa5 100644 --- a/vendor/github.com/securego/gosec/v2/rules/subproc.go +++ b/vendor/github.com/securego/gosec/v2/rules/subproc.go @@ -97,7 +97,7 @@ func (r *subprocess) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) { } // isContext checks whether or not the node is a CommandContext call or not -// Thi is required in order to skip the first argument from the check. +// This is required in order to skip the first argument from the check. func (r *subprocess) isContext(n ast.Node, ctx *gosec.Context) bool { selector, indent, err := gosec.GetCallInfo(n, ctx) if err != nil { -- cgit mrf-deployment