From a26215cf23be85d473f6d5a1e059a02406ea5d55 Mon Sep 17 00:00:00 2001 From: Dimitar Banchev Date: Thu, 29 Aug 2024 17:40:14 +0200 Subject: [PATCH] Migrated the rule to the analyzers folder --- analyzers/hardcodedNonce.go | 231 ++++++++++++++++++++++++++++++++++++ analyzers/util.go | 9 ++ rules/hardcodedIV.go | 136 --------------------- rules/rulelist.go | 1 - rules/rules_test.go | 16 +++ testutils/g407_samples.go | 94 +++++++++++++++ 6 files changed, 350 insertions(+), 137 deletions(-) create mode 100644 analyzers/hardcodedNonce.go delete mode 100644 rules/hardcodedIV.go diff --git a/analyzers/hardcodedNonce.go b/analyzers/hardcodedNonce.go new file mode 100644 index 0000000..2d1d343 --- /dev/null +++ b/analyzers/hardcodedNonce.go @@ -0,0 +1,231 @@ +// (c) Copyright gosec's authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package analyzers + +import ( + "errors" + "fmt" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" + "golang.org/x/tools/go/ssa" + + "github.com/securego/gosec/v2/issue" +) + +const defaultWhat = "Use of hardcoded IV/nonce for encryption" + +func newHardCodedNonce(id string, description string) *analysis.Analyzer { + return &analysis.Analyzer{ + Name: id, + Doc: description, + Run: runHardCodedNonce, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + } +} + +func runHardCodedNonce(pass *analysis.Pass) (interface{}, error) { + ssaResult, err := getSSAResult(pass) + if err != nil { + return nil, fmt.Errorf("building ssa representation: %w", err) + } + + // Holds the function name as key, the number of arguments that the function accepts, and at which index of those accepted arguments is the nonce/IV + // Example "Test" 3, 1 -- means the function "Test" which accepts 3 arguments, and has the nonce arg as second argument + calls := map[string][]int{ + "(crypto/cipher.AEAD).Seal": {4, 1}, + "(crypto/cipher.AEAD).Open": {4, 1}, + "crypto/cipher.NewCBCDecrypter": {2, 1}, + "crypto/cipher.NewCBCEncrypter": {2, 1}, + "crypto/cipher.NewCFBDecrypter": {2, 1}, + "crypto/cipher.NewCFBEncrypter": {2, 1}, + "crypto/cipher.NewCTR": {2, 1}, + "crypto/cipher.NewOFB": {2, 1}, + } + var issues []*issue.Issue + var ssaPkgFunctions = ssaResult.SSA.SrcFuncs + var savedArgsFromFunctions = *iterateAndGetArgsFromTrackedFunctions(ssaPkgFunctions, &calls) + + for _, savedArg := range savedArgsFromFunctions { + tmp, err := raiseIssue(savedArg, &calls, ssaPkgFunctions, pass, "") + issues = append(issues, tmp...) + if err != nil { + return nil, err + } + } + return issues, nil +} + +func raiseIssue(val *ssa.Value, funcsToTrack *map[string][]int, ssaFuncs []*ssa.Function, pass *analysis.Pass, issueDescription string) ([]*issue.Issue, error) { + var err error = nil + var gosecIssue []*issue.Issue = nil + + if issueDescription == "" { + issueDescription = defaultWhat + } + + switch valType := (*val).(type) { + case *ssa.Slice: + issueDescription += " by passing hardcoded slice/array" + tmp, hasErr := iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.High, issue.High) + gosecIssue = append(gosecIssue, tmp...) + err = hasErr + + case *ssa.UnOp: + // Check if it's a dereference operation (a.k.a pointer) + if valType.Op == token.MUL { + issueDescription += " by passing pointer which points to hardcoded variable" + tmp, hasErr := iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.High, issue.Low) + gosecIssue = append(gosecIssue, tmp...) + err = hasErr + } + + // When the value assigned to a variable is a function call. + // It goes and check if this function contains call to crypto/rand.Read in it's body(Assuming that calling crypto/rand.Read in a function, is used for the generation of nonce/iv ) + case *ssa.Call: + if calledFunction, ok := valType.Call.Value.(*ssa.Function); ok { + if contains, funcErr := isFuncContainsCryptoRand(calledFunction); !contains && funcErr != nil { + issueDescription += " by passing a value from function which doesn't use crypto/rand" + tmp, hasErr := iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.High, issue.Medium) + gosecIssue = append(gosecIssue, tmp...) + err = hasErr + } + } + + // only checks from strings->[]byte + // might need to add additional types + case *ssa.Convert: + if valType.Type().String() == "[]byte" && valType.X.Type().String() == "string" { + issueDescription += " by passing converted string" + tmp, hasErr := iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.High, issue.High) + gosecIssue = append(gosecIssue, tmp...) + err = hasErr + } + + case *ssa.Parameter: + //arg given to tracked function is wrapped in another function, example: + // func encrypt(..,nonce,...){ + // aesgcm.Seal() + //} + // save parameter position, by checking the name of the variable used in tracked functions and comparing it with the name of the arg + if valType.Parent() != nil { + trackedFunctions := make(map[string][]int) + for index, funcArgs := range valType.Parent().Params { + if funcArgs.Name() == valType.Name() && funcArgs.Type() == valType.Type() { + trackedFunctions[valType.Parent().String()] = []int{len(valType.Parent().Params), index} + } + } + result := iterateAndGetArgsFromTrackedFunctions(ssaFuncs, &trackedFunctions) + + issueDescription += " by passing a parameter to a function and" + // recursively backtrack to where the origin of a variable passed to multiple functions is + for _, trackedVariable := range *result { + + tmp, hasErr := raiseIssue(trackedVariable, &trackedFunctions, ssaFuncs, pass, issueDescription) + gosecIssue = append(gosecIssue, tmp...) + err = hasErr + } + } + } + return gosecIssue, err +} + +// Iterate through all places that use the `variable` argument and check if it's used in one of the tracked functions +func iterateThroughReferrers(variable *ssa.Value, funcsToTrack *map[string][]int, analyzerID string, issueDescription string, fileSet *token.FileSet, issueSeverity issue.Score, issueConfidence issue.Score) ([]*issue.Issue, error) { + + if funcsToTrack == nil || variable == nil || analyzerID == "" || issueDescription == "" || fileSet == nil { + return nil, errors.New("received a nil object") + } + var gosecIssues []*issue.Issue = nil + // Go trough all functions that use the given arg variable + for _, referrer := range *(*variable).Referrers() { + + // Iterate trough the functions we are interested + for trackedFunc := range *funcsToTrack { + + // Split the functions we are interested in, by the '.' because we will use the function name to do the comparison + // MIGHT GIVE SOME FALSE POSITIVES THIS WAY + trackedFuncParts := strings.Split(trackedFunc, ".") + trackedFuncPartsName := trackedFuncParts[len(trackedFuncParts)-1] + if strings.Contains(referrer.String(), trackedFuncPartsName) { + gosecIssues = append(gosecIssues, newIssue(analyzerID, issueDescription, fileSet, referrer.Pos(), issueSeverity, issueConfidence)) + } + } + } + return gosecIssues, nil +} + +// Check whether a function contains a call to crypto/rand.Read in it's function body +func isFuncContainsCryptoRand(funcCall *ssa.Function) (bool, error) { + if funcCall == nil { + return false, errors.New("passed ssa.Function object is nil") + } + for _, block := range funcCall.Blocks { + for _, instr := range block.Instrs { + if call, ok := instr.(*ssa.Call); ok { + if calledFunction, ok := call.Call.Value.(*ssa.Function); ok { + if calledFunction.Pkg != nil && calledFunction.Pkg.Pkg.Path() == "crypto/rand" && calledFunction.Name() == "Read" { + return true, nil + } + } + } + } + } + return false, nil +} + +func addToVarsMap(value *ssa.Value, mapToAddTo *map[string]*ssa.Value) { + key := (*value).Name() + (*value).Type().String() + (*value).String() + (*value).Parent().String() + (*mapToAddTo)[key] = value +} + +func isContainedInMap(value *ssa.Value, mapToCheck *map[string]*ssa.Value) bool { + key := (*value).Name() + (*value).Type().String() + (*value).String() + (*value).Parent().String() + _, contained := (*mapToCheck)[key] + return contained +} + +func iterateAndGetArgsFromTrackedFunctions(ssaFuncs []*ssa.Function, trackedFunc *map[string][]int) *map[string]*ssa.Value { + values := make(map[string]*ssa.Value) + for _, pkgFunc := range ssaFuncs { + for _, funcBlock := range pkgFunc.Blocks { + for _, funcBlocInstr := range funcBlock.Instrs { + if funcCall, ok := funcBlocInstr.(*ssa.Call); ok { + for trackedFuncName, trackedFuncArgsInfo := range *trackedFunc { + // only process functions that have the same number of arguments as the ones we track + if len(funcCall.Call.Args) == trackedFuncArgsInfo[0] { + tmpArg := funcCall.Call.Args[trackedFuncArgsInfo[1]] + // check if the function is called from an object or directly from the package + if funcCall.Call.Method != nil { + if methodFullname := funcCall.Call.Method.FullName(); methodFullname == trackedFuncName { + if !isContainedInMap(&tmpArg, &values) { + addToVarsMap(&tmpArg, &values) + } + } + } else if funcCall.Call.Value.String() == trackedFuncName { + if !isContainedInMap(&tmpArg, &values) { + addToVarsMap(&tmpArg, &values) + } + } + } + } + } + } + } + } + return &values +} diff --git a/analyzers/util.go b/analyzers/util.go index e88afe7..e7961a5 100644 --- a/analyzers/util.go +++ b/analyzers/util.go @@ -35,6 +35,15 @@ type SSAAnalyzerResult struct { SSA *buildssa.SSA } +// BuildDefaultAnalyzers returns the default list of analyzers +func BuildDefaultAnalyzers() []*analysis.Analyzer { + return []*analysis.Analyzer{ + newConversionOverflowAnalyzer("G115", "Type conversion which leads to integer overflow"), + newSliceBoundsAnalyzer("G602", "Possible slice bounds out of range"), + newHardCodedNonce("G407", "Use of hardcoded IV/nonce for encryption"), + } +} + // getSSAResult retrieves the SSA result from analysis pass func getSSAResult(pass *analysis.Pass) (*SSAAnalyzerResult, error) { result, ok := pass.ResultOf[buildssa.Analyzer] diff --git a/rules/hardcodedIV.go b/rules/hardcodedIV.go deleted file mode 100644 index 2acda68..0000000 --- a/rules/hardcodedIV.go +++ /dev/null @@ -1,136 +0,0 @@ -package rules - -import ( - "go/ast" - - "github.com/securego/gosec/v2" - "github.com/securego/gosec/v2/issue" -) - -type usesHardcodedIV struct { - issue.MetaData - trackedFunctions map[string][]int -} - -func (r *usesHardcodedIV) ID() string { - return r.MetaData.ID -} - -// The code is a little bit spaghetti and there are things that repeat -// Can be improved -func (r *usesHardcodedIV) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) { - // cast n to a call expression, we can do that safely, because this match method gets only called when CallExpr node is found - funcCall := n.(*ast.CallExpr) - - // cast to a function call from an object and get the function part; example: a.doSomething() - - if funcSelector, isSelector := funcCall.Fun.(*ast.SelectorExpr); isSelector { - - // Check if the call is actually made from an object - if _, hasX := funcSelector.X.(*ast.Ident); hasX { - - //Iterate trough the wanted functions - for functionName, functionNumArgsAndNoncePosArr := range r.trackedFunctions { - - // Check if the function name matches with the one we look for, and if the function accepts an exact number of arguments(rough function signature check) - if funcSelector.Sel.Name == functionName && len(funcCall.Args) == functionNumArgsAndNoncePosArr[0] { - - // Check the type of the passed argument to the function - switch trackedFunctionPassedArgType := funcCall.Args[functionNumArgsAndNoncePosArr[1]].(type) { - - // {} used - case *ast.CompositeLit: - // Check if the argument is static array - if _, isArray := trackedFunctionPassedArgType.Type.(*ast.ArrayType); isArray { - return c.NewIssue(n, r.ID(), r.What+" by passing hardcoded byte array", r.Severity, r.Confidence), nil - } - - // () used - case *ast.CallExpr: - - // Check if it's a function call, because []byte() is a function call, and also check if the number of arguments to this call is only 1 - switch trackedFunctionPassedArgType.Fun.(type) { - case *ast.ArrayType: - return c.NewIssue(n, r.ID(), r.What+" by converting static string to a byte array", r.Severity, r.Confidence), nil - - // Check if the argument passed is another function - case *ast.FuncLit: - functionCalled, _ := trackedFunctionPassedArgType.Fun.(*ast.FuncLit) - - // Check the type of the last statement in the anonymous function - switch calledFunctionLastInstructionType := functionCalled.Body.List[len(functionCalled.Body.List)-1].(type) { - - case *ast.IfStmt: - - ifStatementContent := calledFunctionLastInstructionType.Body.List - - // check if the if statement has return statement - if retStatement, isReturn := ifStatementContent[len(ifStatementContent)-1].(*ast.ReturnStmt); isReturn { - argInNestedFunc := retStatement.Results[0] - - // check the type of the returned value - switch argInNestedFunc.(type) { - case *ast.CompositeLit: - // Check if the argument is static array - if _, isArray := argInNestedFunc.(*ast.CompositeLit).Type.(*ast.ArrayType); isArray { - return c.NewIssue(n, r.ID(), r.What+" by passing hardcoded byte array in a function call", r.Severity, r.Confidence), nil - } - - case *ast.CallExpr: - if _, ok := argInNestedFunc.(*ast.CallExpr).Fun.(*ast.ArrayType); ok { - return c.NewIssue(n, r.ID(), r.What+" by converting static string to a byte array in a function call", r.Severity, r.Confidence), nil - } - } - } - case *ast.ReturnStmt: - - argInNestedFunc := calledFunctionLastInstructionType.Results[0] - switch argInNestedFunc.(type) { - case *ast.CompositeLit: - // Check if the argument is static array - if _, isArray := argInNestedFunc.(*ast.CompositeLit).Type.(*ast.ArrayType); isArray { - return c.NewIssue(n, r.ID(), r.What+" by passing hardcoded byte array in a function call", r.Severity, r.Confidence), nil - } - - case *ast.CallExpr: - if _, ok := argInNestedFunc.(*ast.CallExpr).Fun.(*ast.ArrayType); ok { - return c.NewIssue(n, r.ID(), r.What+" by converting static string to a byte array in a function call", r.Severity, r.Confidence), nil - } - } - } - } - } - } - } - } - } - // loop through the functions we are checking - - return nil, nil -} - -func NewUsesHardCodedIV(id string, _ gosec.Config) (gosec.Rule, []ast.Node) { - calls := make(map[string][]int) - // Holds the function name as key, the number of arguments that the function accepts, and at which index of those accepted arguments is the nonce/IV - // Example "Test" 3, 1 -- means the function "Test" which accepts 3 arguments, and has the nonce arg as second argument - - calls["Seal"] = []int{4, 1} - calls["Open"] = []int{4, 1} - calls["NewCBCDecrypter"] = []int{2, 1} // - calls["NewCBCEncrypter"] = []int{2, 1} // - calls["NewCFBDecrypter"] = []int{2, 1} - calls["NewCFBEncrypter"] = []int{2, 1} - calls["NewCTR"] = []int{2, 1} // - calls["NewOFB"] = []int{2, 1} // - - rule := &usesHardcodedIV{ - trackedFunctions: calls, - MetaData: issue.MetaData{ - ID: id, - Severity: issue.High, - Confidence: issue.Medium, - What: "Use of hardcoded IV/nonce for encryption", - }, - } - return rule, []ast.Node{(*ast.CallExpr)(nil)} -} diff --git a/rules/rulelist.go b/rules/rulelist.go index 889413c..13f29f7 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -100,7 +100,6 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList { {"G404", "Insecure random number source (rand)", NewWeakRandCheck}, {"G405", "Detect the usage of DES or RC4", NewUsesWeakCryptographyEncryption}, {"G406", "Detect the usage of deprecated MD4 or RIPEMD160", NewUsesWeakDeprecatedCryptographyHash}, - {"G407", "Detect the usage of hardcoded Initialization Vector(IV)/Nonce", NewUsesHardCodedIV}, // blocklist {"G501", "Import blocklist: crypto/md5", NewBlocklistedImportMD5}, diff --git a/rules/rules_test.go b/rules/rules_test.go index 783ab87..036342a 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -247,6 +247,22 @@ var _ = Describe("gosec rules", func() { runner("G407", testutils.SampleCodeG407o) }) + It("should detect hardcoded nonce/IV", func() { + runner("G407", testutils.SampleCodeG407p) + }) + + It("should detect hardcoded nonce/IV", func() { + runner("G407", testutils.SampleCodeG407q) + }) + + It("should detect hardcoded nonce/IV", func() { + runner("G407", testutils.SampleCodeG407r) + }) + + It("should detect hardcoded nonce/IV", func() { + runner("G407", testutils.SampleCodeG407s) + }) + It("should detect blocklisted imports - MD5", func() { runner("G501", testutils.SampleCodeG501) }) diff --git a/testutils/g407_samples.go b/testutils/g407_samples.go index 675aa06..6bd6c2a 100644 --- a/testutils/g407_samples.go +++ b/testutils/g407_samples.go @@ -377,4 +377,98 @@ func main() { } `}, 2, gosec.NewConfig()}, } + + // SampleCodeG407p - Use of hardcoded nonce/IV + SampleCodeG407p = []CodeSample{ + {[]string{`package main + +import ( + "crypto/aes" + "crypto/cipher" + "fmt" +) + +func main() { + + var nonce = []byte("ILoveMyNonce") + block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) + aesGCM, _ := cipher.NewGCM(block) + fmt.Println(string(aesGCM.Seal(nil, nonce, []byte("My secret message"), nil))) +} +`}, 1, gosec.NewConfig()}, + } + + // SampleCodeG407q - Use of hardcoded nonce/IV + SampleCodeG407q = []CodeSample{ + {[]string{`package main + +import ( + "crypto/aes" + "crypto/cipher" + "fmt" +) + +func main() { + + var nonce = []byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1} + block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) + aesCTR := cipher.NewCTR(block, nonce) + var output = make([]byte, 16) + aesCTR.XORKeyStream(output, []byte("Very Cool thing!")) + fmt.Println(string(output)) +} +`}, 1, gosec.NewConfig()}, + } + + // SampleCodeG407r - Use of hardcoded nonce/IV + SampleCodeG407r = []CodeSample{ + {[]string{`package main + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "fmt" +) + +func coolFunc(size int) []byte{ + buf := make([]byte, size) + rand.Read(buf) + return buf +} + +func main() { + + var nonce = coolFunc(16) + block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) + aesCTR := cipher.NewCTR(block, nonce) + var output = make([]byte, 16) + aesCTR.XORKeyStream(output, []byte("Very Cool thing!")) + fmt.Println(string(output)) +} +`}, 0, gosec.NewConfig()}, + } + // SampleCodeG407s - Use of hardcoded nonce/IV + SampleCodeG407s = []CodeSample{ + {[]string{`package main + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "fmt" +) + +var nonce = []byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1} + +func main() { + + block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) + aesGCM, _ := cipher.NewGCM(block) + cipherText := aesGCM.Seal(nil, nonce, []byte("My secret message"), nil) + fmt.Println(string(cipherText)) + +} +`}, 1, gosec.NewConfig()}, + } )