praetorian-inc / gokart

A static analysis tool for securing Go code
Apache License 2.0
2.18k stars 110 forks source link

RSA warnings are suppressed in output #86

Open glinton opened 1 year ago

glinton commented 1 year ago

Result

Race Complete! Analysis took 84.813012ms and 24 Go files were scanned (including imported packages)
GoKart found 0 potentially vulnerable functions

Expected

(CWE-326: Inadequate Encryption Strength) Danger: RSA key length is too short, recommend 2048

To Reproduce

# create temporary directory
mkdir -p /tmp/rsa

# create minimal go file also at (https://play.golang.com/p/1Js9F91OGDk.go)
cat > /tmp/rsa/main.go <<-"EOF"
package main

import (
        "crypto/rand"
        "crypto/rsa"
)

func main() {
        rsa.GenerateKey(rand.Reader, 100)
}
EOF

# make module so gokart runs in directory
cat > /tmp/rsa/go.mod <<-EOF
module demo.thing/rsa

go 1.19
EOF

# scan with gokart
gokart scan /tmp/rsa/

Exploration

Since https://github.com/praetorian-inc/gokart/blob/4b43e9f80dde4198e8b6dcb93ea8ae6c554c4162/analyzers/rsa.go#L178 is always passing nil as the untrusted_source, and https://github.com/praetorian-inc/gokart/blob/a5aab4cabae53a23c91dc61e8f0b73709625065e/util/finding.go#L54 is always considering 0 length Untrusted_Source's to be an invalid finding, the actual notice gets filterred out.

There are no unit tests for util/finding.go or integration tests for analyzers/scan.go, so this is missed (since the tests in analyzers/scan.go only test that the returned results contains the expected number of results, not the correct number of "ValidFindings").

Workaround

I found that by adding []util.TaintedCode{{SourceCode: vulnFunc.Instr.Call.Value.String()}} to the result, that gokart prints it. It doesn't feel entirely correct since it's not necessarily "Untrusted Input", but at least it is bringing attention to it (hence the workaround, not actual fix).

New Output

(CWE-326: Inadequate Encryption Strength) Danger: RSA key length is too short, recommend 2048

/tmp/rsa/main.go:9
Vulnerable Function: [ main(...) ]
      8:    func main() {
    > 9:            rsa.GenerateKey(rand.Reader, 100)
      10:   }

:0
Source of Untrusted Input: [ (...) ]
      -1:   
    > 0:    crypto/rsa.GenerateKey
      1:    
------------------------------------------------------------------------------

Race Complete! Analysis took 81.617933ms and 24 Go files were scanned (including imported packages)
GoKart found 1 potentially vulnerable functions
Identified 1 potential CWE-326: Inadequate Encryption Strength

Patch

-results = append(results, util.MakeFinding(message, targetFunc, nil, "CWE-326: Inadequate Encryption Strength"))
+results = append(results, util.MakeFinding(message, targetFunc, []util.TaintedCode{{SourceCode: vulnFunc.Instr.Call.Value.String()}}, "CWE-326: Inadequate Encryption Strength"))