Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

RSA warnings are suppressed in output #86

Open
glinton opened this issue Jan 13, 2023 · 0 comments
Open

RSA warnings are suppressed in output #86

glinton opened this issue Jan 13, 2023 · 0 comments

Comments

@glinton
Copy link

glinton commented Jan 13, 2023

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

results = append(results, util.MakeFinding(message, targetFunc, nil, "CWE-326: Inadequate Encryption Strength"))
is always passing nil as the untrusted_source, and
if len(finding.Untrusted_Source) == 0 {
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"))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

1 participant