Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new rule G407 to detect hard-coded nonce and initialization vectors in crypto algorithms #1197

Merged
merged 13 commits into from
Aug 30, 2024
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ directory you can supply `./...` as the input argument.
- G404: Insecure random number source (rand)
- G405: Detect the usage of DES or RC4
- G406: Detect the usage of MD4 or RIPEMD160
- G407: Detect the usage of hardcoded Initialization Vector(IV)/Nonce
- G501: Import blocklist: crypto/md5
- G502: Import blocklist: crypto/des
- G503: Import blocklist: crypto/rc4
Expand Down
38 changes: 38 additions & 0 deletions analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,44 @@ var _ = Describe("Analyzer", func() {
Expect(issues[0].Suppressions[0].Justification).To(Equal("Globally suppressed."))
})

It("should not report an error if the analyzer is not included", func() {
sample := testutils.SampleCodeG407[0]
source := sample.Code[0]
analyzer.LoadAnalyzers(analyzers.Generate(true, analyzers.NewAnalyzerFilter(false, "G115")).AnalyzersInfo())

controlPackage := testutils.NewTestPackage()
defer controlPackage.Close()
controlPackage.AddFile("cipher.go", source)
err := controlPackage.Build()
Expect(err).ShouldNot(HaveOccurred())
err = analyzer.Process(buildTags, controlPackage.Path)
Expect(err).ShouldNot(HaveOccurred())
controlIssues, _, _ := analyzer.Report()
Expect(controlIssues).Should(HaveLen(sample.Errors))
Expect(controlIssues[0].Suppressions).To(HaveLen(1))
Expect(controlIssues[0].Suppressions[0].Kind).To(Equal("external"))
Expect(controlIssues[0].Suppressions[0].Justification).To(Equal("Globally suppressed."))
})

It("should not report an error if the analyzer is excluded", func() {
sample := testutils.SampleCodeG407[0]
source := sample.Code[0]
analyzer.LoadAnalyzers(analyzers.Generate(true, analyzers.NewAnalyzerFilter(true, "G407")).AnalyzersInfo())

controlPackage := testutils.NewTestPackage()
defer controlPackage.Close()
controlPackage.AddFile("cipher.go", source)
err := controlPackage.Build()
Expect(err).ShouldNot(HaveOccurred())
err = analyzer.Process(buildTags, controlPackage.Path)
Expect(err).ShouldNot(HaveOccurred())
issues, _, _ := analyzer.Report()
Expect(issues).Should(HaveLen(sample.Errors))
Expect(issues[0].Suppressions).To(HaveLen(1))
Expect(issues[0].Suppressions[0].Kind).To(Equal("external"))
Expect(issues[0].Suppressions[0].Justification).To(Equal("Globally suppressed."))
})

It("should not report an error if the analyzer is not included", func() {
sample := testutils.SampleCodeG602[0]
source := sample.Code[0]
Expand Down
4 changes: 4 additions & 0 deletions analyzers/analyzers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ var _ = Describe("gosec analyzers", func() {
runner("G115", testutils.SampleCodeG115)
})

It("should detect hardcoded nonce/IV", func() {
runner("G407", testutils.SampleCodeG407)
})

It("should detect out of bounds slice access", func() {
runner("G602", testutils.SampleCodeG602)
})
Expand Down
1 change: 1 addition & 0 deletions analyzers/analyzerslist.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func NewAnalyzerFilter(action bool, analyzerIDs ...string) AnalyzerFilter {
var defaultAnalyzers = []AnalyzerDefinition{
{"G115", "Type conversion which leads to integer overflow", newConversionOverflowAnalyzer},
{"G602", "Possible slice bounds out of range", newSliceBoundsAnalyzer},
{"G407", "Use of hardcoded IV/nonce for encryption", newHardCodedNonce},
}

// Generate the list of analyzers to use
Expand Down
246 changes: 246 additions & 0 deletions analyzers/hardcodedNonce.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
// (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 defaultIssueDescription = "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},
}
ssaPkgFunctions := ssaResult.SSA.SrcFuncs
args := getArgsFromTrackedFunctions(ssaPkgFunctions, calls)
if args == nil {
return nil, errors.New("no tracked functions found, resulting in no variables to track")
}
var issues []*issue.Issue
for _, arg := range args {
if arg == nil {
continue
}
i, err := raiseIssue(*arg, calls, ssaPkgFunctions, pass, "")
if err != nil {
expp121 marked this conversation as resolved.
Show resolved Hide resolved
return issues, fmt.Errorf("raising issue error: %w", err)
}
issues = append(issues, i...)
}
return issues, nil
}

func raiseIssue(val ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.Function,
pass *analysis.Pass, issueDescription string,
) ([]*issue.Issue, error) {
if issueDescription == "" {
issueDescription = defaultIssueDescription
}
var err error
var allIssues []*issue.Issue
var issues []*issue.Issue
switch valType := (val).(type) {
case *ssa.Slice:
issueDescription += " by passing hardcoded slice/array"
issues, err = iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.High)
allIssues = append(allIssues, issues...)
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"
issues, err = iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.Low)
allIssues = append(allIssues, issues...)
}
// 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 callValue := valType.Call.Value; callValue != nil {
if calledFunction, ok := callValue.(*ssa.Function); ok {
if contains, funcErr := isFuncContainsCryptoRand(calledFunction); !contains && funcErr == nil {
issueDescription += " by passing a value from function which doesn't use crypto/rand"
issues, err = iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.Medium)
allIssues = append(allIssues, issues...)
} else if funcErr != nil {
err = funcErr
}
}
}
// 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"
issues, err = iterateThroughReferrers(val, funcsToTrack, pass.Analyzer.Name, issueDescription, pass.Fset, issue.High)
allIssues = append(allIssues, issues...)
}
case *ssa.Parameter:
// arg given to tracked function is wrapped in another function, example:
// func encrypt(..,nonce,...){
// aesgcm.Seal(nonce)
// }
// 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}
}
}
args := getArgsFromTrackedFunctions(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 _, arg := range args {
if arg == nil {
continue
}
issues, err = raiseIssue(*arg, trackedFunctions, ssaFuncs, pass, issueDescription)
allIssues = append(allIssues, issues...)
}
}
}
return allIssues, err
}

// iterateThroughReferrers iterates 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, 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
refs := variable.Referrers()
if refs == nil {
return gosecIssues, nil
}
// Go trough all functions that use the given arg variable
for _, ref := range *refs {
// 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(ref.String(), trackedFuncPartsName) {
gosecIssues = append(gosecIssues, newIssue(analyzerID, issueDescription, fileSet, ref.Pos(), issue.High, issueConfidence))
}
}
}
return gosecIssues, nil
}

// isFuncContainsCryptoRand checks 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 getArgsFromTrackedFunctions(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 {
iterateTrackedFunctionsAndAddArgs(funcBlocInstr, trackedFunc, values)
}
}
}
return values
}

func iterateTrackedFunctionsAndAddArgs(funcBlocInstr ssa.Instruction, trackedFunc map[string][]int, values map[string]*ssa.Value) {
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)
}
}
}
}
}
}
9 changes: 9 additions & 0 deletions analyzers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
5 changes: 5 additions & 0 deletions cwe/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ var idWeaknesses = map[string]*Weakness{
Description: "The software contains hard-coded credentials, such as a password or cryptographic key, which it uses for its own inbound authentication, outbound communication to external components, or encryption of internal data.",
Name: "Use of Hard-coded Credentials",
},
"1204": {
ID: "1204",
Description: "The product uses a cryptographic primitive that uses an Initialization Vector (IV), but the product does not generate IVs that are sufficiently unpredictable or unique according to the expected cryptographic requirements for that primitive.",
Name: "Generation of Weak Initialization Vector (IV)",
},
}

// Get Retrieves a CWE weakness by it's id
Expand Down
1 change: 1 addition & 0 deletions issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ var ruleToCWE = map[string]string{
"G404": "338",
"G405": "327",
"G406": "328",
"G407": "1204",
"G501": "327",
"G502": "327",
"G503": "327",
Expand Down
4 changes: 2 additions & 2 deletions report/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ var _ = Describe("Formatter", func() {
"G101", "G102", "G103", "G104", "G106", "G107", "G109",
"G110", "G111", "G112", "G113", "G201", "G202", "G203",
"G204", "G301", "G302", "G303", "G304", "G305", "G401",
"G402", "G403", "G404", "G405", "G406", "G501", "G502",
"G503", "G504", "G505", "G506", "G507", "G601",
"G402", "G403", "G404", "G405", "G406", "G407", "G501",
"G502", "G503", "G504", "G505", "G506", "G507", "G601",
}

It("csv formatted report should contain the CWE mapping", func() {
Expand Down
Loading
Loading