Skip to content

Commit

Permalink
Pass the value argument directly since is an interface
Browse files Browse the repository at this point in the history
The value doens't require to be passed as a pointer since is a
interface.

Change-Id: Ia21bceb5f315f4c30bd28425d62f678e9203e93f
Signed-off-by: Cosmin Cojocar <ccojocar@google.com>
ccojocar committed Aug 30, 2024

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
1 parent f5d3128 commit 2401936
Showing 1 changed file with 38 additions and 39 deletions.
77 changes: 38 additions & 39 deletions analyzers/hardcodedNonce.go
Original file line number Diff line number Diff line change
@@ -64,7 +64,10 @@ func runHardCodedNonce(pass *analysis.Pass) (interface{}, error) {
}

for _, savedArg := range savedArgsFromFunctions {
tmp, err := raiseIssue(savedArg, calls, ssaPkgFunctions, pass, "")
if savedArg == nil {
continue
}
tmp, err := raiseIssue(*savedArg, calls, ssaPkgFunctions, pass, "")
if err != nil {
return issues, fmt.Errorf("raising issue error: %w", err)
}
@@ -73,24 +76,21 @@ func runHardCodedNonce(pass *analysis.Pass) (interface{}, error) {
return issues, nil
}

func raiseIssue(val *ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.Function, pass *analysis.Pass, issueDescription string) ([]*issue.Issue, error) {
if *val == nil {
return nil, errors.New("received a pointer to a <nil> ssa.Value")
}
func raiseIssue(val ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.Function,
pass *analysis.Pass, issueDescription string) ([]*issue.Issue, error) {
var err error
var gosecIssue []*issue.Issue

if issueDescription == "" {
issueDescription = defaultIssueDescription
}

switch valType := (*val).(type) {
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)
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 {
@@ -99,7 +99,6 @@ func raiseIssue(val *ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.F
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,
@@ -117,7 +116,6 @@ func raiseIssue(val *ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.F
}
}
}

// only checks from strings->[]byte
// might need to add additional types
case *ssa.Convert:
@@ -127,7 +125,6 @@ func raiseIssue(val *ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.F
gosecIssue = append(gosecIssue, tmp...)
err = hasErr
}

case *ssa.Parameter:
// arg given to tracked function is wrapped in another function, example:
// func encrypt(..,nonce,...){
@@ -147,7 +144,10 @@ func raiseIssue(val *ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.F
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)
if trackedVariable == nil {
continue
}
tmp, hasErr := raiseIssue(*trackedVariable, trackedFunctions, ssaFuncs, pass, issueDescription)
gosecIssue = append(gosecIssue, tmp...)
err = hasErr
}
@@ -157,27 +157,29 @@ func raiseIssue(val *ssa.Value, funcsToTrack map[string][]int, ssaFuncs []*ssa.F
}

// 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,
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
if varReferrers := (*variable).Referrers(); *varReferrers != nil {
for _, referrer := range *varReferrers {
// 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(), issue.High, issueConfidence))
}
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))
}
}
}
@@ -203,16 +205,13 @@ func isFuncContainsCryptoRand(funcCall *ssa.Function) (bool, error) {
return false, nil
}

func addToVarsMap(value *ssa.Value, mapToAddTo map[string]*ssa.Value) {
valDerefed := *value
key := valDerefed.Name() + valDerefed.Type().String() + valDerefed.String() + valDerefed.Parent().String()
mapToAddTo[key] = value
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 {
valDerefed := *value

key := valDerefed.Name() + valDerefed.Type().String() + valDerefed.String() + valDerefed.Parent().String()
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
}
@@ -222,29 +221,29 @@ func iterateAndGetArgsFromTrackedFunctions(ssaFuncs []*ssa.Function, trackedFunc
for _, pkgFunc := range ssaFuncs {
for _, funcBlock := range pkgFunc.Blocks {
for _, funcBlocInstr := range funcBlock.Instrs {
iterateTrackedFunctionsAndAddArgs(&funcBlocInstr, trackedFunc, values)
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 {
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)
if !isContainedInMap(tmpArg, values) {
addToVarsMap(tmpArg, values)
}
}
} else if funcCall.Call.Value.String() == trackedFuncName {
if !isContainedInMap(&tmpArg, values) {
addToVarsMap(&tmpArg, values)
if !isContainedInMap(tmpArg, values) {
addToVarsMap(tmpArg, values)
}
}
}

0 comments on commit 2401936

Please sign in to comment.